Re: [PATCH] fs: fix i_writecount on shmem and friends

[NeilBrown]

On Thu, 13 Mar 2014 04:29:34 +0000 Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:

> On Thu, Mar 13, 2014 at 03:08:00PM +1100, NeilBrown wrote:
> > +		inode = mddev->bitmap_info.file->f_mapping->host;
> > +		if (!S_ISREG(inode->i_mode)) {
> > +			printk(KERN_ERR "%s: error: bitmap file must be a regular file\n",
> > +			       mdname(mddev));
> > +			fput(mddev->bitmap_info.file);
> > +			mddev->bitmap_info.file = NULL;
> > +			return -EBADF;
> > +		}
> > +		if (atomic_read(&inode->i_writecount) != 1) {
> 
> Umm...  I think you ought to check more than that.  At the very least you
> want to check that you have it opened for write - you don't want e.g.
> a filesystem containing that puppy remounted r/o under you.  Another thing
> is, what happens if it's not a buffer cache backed one?  Hell, what happens
> if it's a file on NFS?  You are relying on bmap() working, right?  So it
> looks like you ought to check if ->bmap() is there.  And I really wonder
> how well does it play with journalling fs...

Can we do direct writes from kernel space yet?  If so I'll change the code to
do that so that it will work with any filesystem (which supports direct
writes).
(The documentation says we that bitmap files should only be used on ext2 or
ext3.  Most people use bitmaps on the raw devices so hopefully the few who
have a need for files will read the documentation :-)

(and yes, I check for FMODE_WRITE)

Thanks,
NeilBrown

Attachment: signature.asc
Description: PGP signature