On Tue, Apr 21, 2015 at 03:49:59PM +0100, Al Viro wrote: > On Mon, Apr 20, 2015 at 07:12:22PM +0100, Al Viro wrote: > > > Patches 2/24..6/24 are from Neil's RCU follow_link patchset; the > > rest of his patchset is, of course, derailed by the massage done here, > > but AFAICS we should be able to port it on top of this one with reasonably > > little PITA. > > BTW, looking at the ->put_link() instances in the tree, after this series > all but one of them ignore *everything* other than cookie. The only exception > is hppfs; it wants dentry (and its inode as well): > > static void hppfs_put_link(struct dentry *dentry, void *cookie) > { > struct dentry *proc_dentry = HPPFS_I(d_inode(dentry))->proc_dentry; > > if (d_inode(proc_dentry)->i_op->put_link) > d_inode(proc_dentry)->i_op->put_link(proc_dentry, cookie); > } The hppfs code looks totally bogus in general. Richard, do you know if anyone still uses that part of UML? > Does anybody have objections against passing them inodes instead of dentries? > It would be a lot more convenient for RCU purposes... Sounds fine to me. > * XFS: is there any reason why you guys don't separate inline and > non-inline symlinks? Or don't just use page_follow_link_light() for the > latter... Historical reasons, feel free to split it up. > Note, BTW, that NUL-termination for the latter will be taken > care of by page_getlink(). It's inline ones that look like crap, actually - > unless I'm misreading that code, the longest possible inline symlink will > have ip->i_df.if_u1.if_data completely filled, with no place for terminating > NUL. Is that correct? If so, it might make sense to have three variants - > short (== inline shorter than XFS_IFORK_DSIZE) just having ->follow_link() > return ip->i_df.if_u1.if_data and have xfs_setup_inode() do nd_terminate_link() > to make sure they are NUL-terminated, long (non-inline) just using > page_follow_link_light() and bogus (inline with length exactly equal to > XFS_IFORK_DSIZE); the last would be the only ones with XFS-specific non-trivial > ->follow_link() - those really need to allocate a buffer and copy into it... In theory we could allocate if_data to include th terminator in memory. I'd have to see how ugly that would be. > * logfs has ->follow_link equal to page_follow_link_light and > NULL ->put_link. Obvious leak, and that one's -stable fodder - it had > been there all way back to original merge. I'll send a fix in a minute. logfs has been unmaintained since 2011, we might as well drop it.. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html