lkml.org 
[lkml]   [2011]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/5] fsnotify: grab inode ref in add_inode_mark() instead of add_mark()
On Fri, Feb 25, 2011 at 01:36:35PM -0500, Eric Paris wrote:
> I think your patch series is making a noticeable change which I don't
> think I'm comfortable with. The current code does not pin inodes in
> core if they only have ignored masks. Under memory pressure those
> inodes can get eviced and the mark will get cleaned up. My glance at
> this code leads me to believe that all inodes with any mark is going to
> be pinned in core. I don't think that's a good idea for AV vendor use
> where they might want to watch everything on the system with mount point
> marks and put ignored marks on everything that comes along to cache
> allows.
>
> The fact that inodes might not be pinned in core is the reason for some
> of the stupid difficulty. There is probably some way to work it out but
> I think it's something I'm going to need to think about.

Youre right, the inode is now also pinned if there is no mask set. This is
a change i did not on purpose. Whether the inode is pinned or not does not
affect the original purpose of the patch series, which was the decoupling
of the mark lock and the fsobject lock. I simply forgot to check the mask.
I could replace that part with something like

- mark->i.inode = igrab(inode);
- mark->flags |= FSNOTIFY_MARK_FLAG_OBJECT_PINNED;
+ mark->i.inode = inode;
+ if (mark->mask) { /* only pin if mask is set */
+ igrab(inode);
+ mark->flags |= FSNOTIFY_MARK_FLAG_OBJECT_PINNED;
+ }
Should i just fix it and resend the patches? Or do you have any other
concerns?


\
 
 \ /
  Last update: 2011-03-01 19:37    [from the cache]
©2003-2011 Jasper Spaans