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

2014-03-20 Thread David Herrmann
Hi On Thu, Mar 13, 2014 at 1:37 AM, Al Viro wrote: > We do, but how do you get anything even attempt deny_write_access() on > those? And what would the semantics of that be, anyway? I just discovered /proc/self/map_files/ (only enabled with CONFIG_CHECKPOINT_RESTORE). Attached is an example to

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

2014-03-13 Thread Al Viro
On Thu, Mar 13, 2014 at 04:55:41PM +1100, NeilBrown wrote: > 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). You can - see __swap_writepage() (mm/page_io.c). However, that area is a

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

2014-03-13 Thread David Herrmann
Hi Al On Thu, Mar 13, 2014 at 1:37 AM, Al Viro wrote: > On Wed, Mar 12, 2014 at 11:30:09PM +0100, David Herrmann wrote: >> Please see: >> shmem_zero_setup() >> shmem_file_setup() >> __shmem_file_setup() >> alloc_file() >> >> shmem_zero_setup() is used by /dev/zero (drivers/cha

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

2014-03-12 Thread NeilBrown
On Thu, 13 Mar 2014 04:29:34 + Al Viro 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

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

2014-03-12 Thread Al Viro
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", > +md

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

2014-03-12 Thread NeilBrown
On Wed, 12 Mar 2014 18:19:25 + Al Viro wrote: > On Tue, Mar 11, 2014 at 12:05:09PM -0700, Linus Torvalds wrote: > > > > which returns ETXTBSY (most easily seen by just stracing it). > > > > The patch would also seem to make sense, with the i_readcount_inc() > > being immediately below for t

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

2014-03-12 Thread Al Viro
On Wed, Mar 12, 2014 at 11:30:09PM +0100, David Herrmann wrote: > Hi > > > I think it's trying to fix the problem in the wrong place. The bug is real, > > all right, but it's not that alloc_file() for non-regulars doesn't grab > > writecount; it's that drop_file_write_access() drops it for those.

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

2014-03-12 Thread David Herrmann
Hi > I think it's trying to fix the problem in the wrong place. The bug is real, > all right, but it's not that alloc_file() for non-regulars doesn't grab > writecount; it's that drop_file_write_access() drops it for those. > > What the hell would we want to play with that counter for, anyway? I

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

2014-03-12 Thread Al Viro
On Tue, Mar 11, 2014 at 12:05:09PM -0700, Linus Torvalds wrote: > > which returns ETXTBSY (most easily seen by just stracing it). > > The patch would also seem to make sense, with the i_readcount_inc() > being immediately below for the FMODE_READ case. I think it's trying to fix the problem in t

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

2014-03-11 Thread Linus Torvalds
Al, any comments? David's test-program is some broken mix of C and shell scripting, but the fixed version does show the issue he talks about: int main(int argc, char **argv) { int p[2], ro; char buf[128]; pipe(p); sprintf(buf, "/proc/self/f

[PATCH] fs: fix i_writecount on shmem and friends

2014-03-03 Thread David Herrmann
VM_DENYWRITE currently relies on i_writecount. Unless there's an active writable reference to an inode, VM_DENYWRITE is not allowed. Unfortunately, alloc_file() does not increase i_writecount, therefore, does not prevent a following VM_DENYWRITE even though the new file might have been opened with