Re: fs_stack/eCryptfs: remove 3rd arg of copy_attr_all, add lockingto copy_inode_size

From: Hugh Dickins
Date: Thu Apr 03 2008 - 15:02:25 EST


On Thu, 3 Apr 2008, Josef 'Jeff' Sipek wrote:
> On Wed, Apr 02, 2008 at 09:49:11PM -0400, Erez Zadok wrote:
> ...
> > +#if BITS_PER_LONG == 32 && defined(CONFIG_SMP)
> > + spin_lock(&dst->i_lock);
> > +#endif
>
> I think you need to check CONFIG_PREEMPT as well.

Don't i_size_read() and i_size_write() handle that case adequately
themselves with preempt_dis/enable?

If you mean it'd be better to have the preempt_dis/enable bracketing
around the i_size_read() along with the i_size_write(): well, could
do, but if lower i_size is changing underneath us in that way,
it can just as well change again a moment later, so no point.

The bit that worries me, that prevents me from saying Acked-by,
is that I'm unsure of the extent to which lower filesystems are
relying on i_mutex or something else for their i_size serialization.
Using the i_lock here plays well with unionfs internally, and fixes
the hangs I used to see; but relying on one locking scheme here and
another below is uncomfortable for me. But it is an improvement.

Hugh
--
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/