Re: [PATCH 0/6] fix INT_MAX readdir hang, plus cleanups

[Chris Mason <clmason@fusionio.com>]

Quoting David Sterba (2013-06-06 09:55:50)
> On Thu, Jun 06, 2013 at 09:35:07AM +0800, Miao Xie wrote:
> > On    wed, 5 Jun 2013 15:36:36 +0200, David Sterba wrote:
> > > On Wed, Jun 05, 2013 at 10:34:08AM +0800, Miao Xie wrote:
> > >> On         tue, 4 Jun 2013 16:26:57 -0700, Zach Brown wrote:
> > >>> On Tue, Jun 04, 2013 at 07:16:53PM -0400, Chris Mason wrote:
> > >>>> Quoting Zach Brown (2013-06-04 18:17:54)
> > >>>>> Hi gang,
> > >>>>>
> > >>>>> I finally sat down to fix that readdir hang that has been in the back
> > >>>>> of my mind for a while.  I *hope* that the fix is pretty simple: just
> > >>>>> don't manufacture a fake f_pos, I *think* we can abuse f_version as an
> > >>>>> indicator that we shouldn't return entries.  Does this look reasonable?
> > >>>>
> > >>>> I like it, and it doesn't look too far away from how others are abusing
> > >>>> f_version.  Have you tried with NFS?  I don't think it'll hurt, but NFS
> > >>>> loves to surprise me.
> > >>>
> > >>> Mm, no, I hadn't.  I'll give it a go tomorrow.  What could go wrong? :)
> > >>
> > >> If we can not use f_version, we can use private_data. I think this variant is
> > >> safe.
> > > 
> > > private_data is used within the ioctl user transactions, so a
> > > readdir(mountpoint) with a user transaction running can break it.
> > 
> > don't worry, we can allocate a structure to keep both transaction handle and the information
> > of readdir, just like ext3/ext4. It is a flexible way and we can extend the structure to keep
> > more information if need in the future.
> > 
> > Beside the above method, we also can abuse the low bits of private_data to indicator that
> > we shouldn't return entries.
> 
> Allocating a full structure for private_data sounds better than directly
> modifying the pointer value itself.

I'd actually rather tag the pointers than go through kmalloc, we just
need one bit (maybe that really just shows how badly we've corrupted
poor Miao).  But, we're not there yet, I think Zach's initial patch will
work fine.

-chris