Messages in this thread | | | Date | Thu, 3 Apr 2008 20:02:12 +0100 (BST) | From | Hugh Dickins <> | Subject | Re: fs_stack/eCryptfs: remove 3rd arg of copy_attr_all, add locking to copy_inode_size |
| |
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
| |