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

[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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/