Messages in this thread | | | Date | Wed, 12 Mar 2014 23:30:09 +0100 | Subject | Re: [PATCH] fs: fix i_writecount on shmem and friends | From | 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? It's not > as if they could be mmapped, so all it does is making pipe(2) and socket(2) > more costly, for no visible reason.
Please see: shmem_zero_setup() shmem_file_setup() __shmem_file_setup() alloc_file()
shmem_zero_setup() is used by /dev/zero (drivers/char/mem.c) and mmap(MAP_ANON). So yes, we do call mmap() on these files. I also disagree on "for no visible reason". The reason to do this is uniformity. We make i_writecount work on all inodes regardless how they got created. Breaking consistent behavior just to save an atomic_inc_unless_negative() in the alloc_file() path seems unreasonable to me.
Anyhow, I haven't found any bug if we follow your recommendation. Every path using alloc_file() either prevents write access on the underlying inode (eg., anon-inode) or prevents user-space from getting an FD (all the shmem_file_setup() paths). So it's up to you. Feel free to fix it yourself, otherwise I will send a second patch following your idea tomorrow.
Oh, and yes, the md stuff is broken, but I thought that's an orthogonal issue..
Thanks David
| |