Re: [PATCH] Busy inodes after unmount, be more verbose in generic_shutdown_super

[Neil Brown <neilb@suse.de>]

On Monday March 6, jblunck@suse.de wrote:
> 
> This are two different problems which you adress with this and your first
> patch. This one is to prevent busy inodes on umouny, the first one was to get
> the reference counting on dentries right.

I think that solving the "busy inodes" problem is sufficient.  The
reference count on dentries isn't *wrong* as someone is actually
holding a reference.  It is just that generic_shutdown_super doesn't
expect anyone else to hold any references.  Fixing the "busy inodes"
problem means that no-one else will be holding any references, so it
becomes a non-problem.
> 
> Neil, did you actually read my patch for this one?!
> http://marc.theaimsgroup.com/?l=linux-kernel&m=114123870406751&w=2

No, I didn't :-(  I obviously didn't do enough homework.

The significant differences seem to be:
 - you test ->s_prunes inside the spinlock.  I don't bother.  Yours is
   probably safer.
 - You call wake_up every time through prune_one_dentry while I try to
   limit the calls.  As each call is a function call and a spinlock,
   maybe at least guard it with
      if(waitqueue_active()) ...


> 
> What I don't like, is that you are serializing the work of shrink_dcache_*
> although they could work in parallel on different processors.

I don't see how I am parallelising anything.  Multiple shrink_dcache_*
can still run.  The only place that extra locking is done is in
generic_shutdown_super.

But what do you think of Balbir Singh's patch?  I think it is less
intrusive and solves the problem just a well.

Thanks,
NeilBrown


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