On Sat, Jul 15, 2017 at 04:36:27PM +0000, Tinguely, Mark wrote: > -----Original Message----- > From: linux-xfs-owner@xxxxxxxxxxxxxxx [mailto:linux-xfs-owner@xxxxxxxxxxxxxxx] On Behalf Of Darrick J. Wong > Sent: Friday, July 14, 2017 5:47 PM > To: Brian Foster <bfoster@xxxxxxxxxx> > Cc: Eric Sandeen <sandeen@xxxxxxxxxxx>; Allison Henderson <allison.henderson@xxxxxxxxxx>; cmaiolino@xxxxxxxxxx; linux-xfs@xxxxxxxxxxxxxxx > Subject: Re: Parent pointers > > On Fri, Jul 14, 2017 at 03:07:41PM -0400, Brian Foster wrote: > > On Fri, Jul 14, 2017 at 01:46:27PM -0500, Eric Sandeen wrote: > > > On 07/14/2017 12:44 PM, Allison Henderson wrote: > > > > On 7/14/2017 7:04 AM, Eric Sandeen wrote: > > > >> > > <parent inode pointers returneth messages deleted> > > > It's something to keep in mind, in any event. IIRC there were also > > missing Signed-off-by's required for some of Mark's original patches. > > That's also a problem; unless someone can get Mark to supply them, we probably have to get someone to rewrite them. At a bare minimum I think we explicitly have to pass back a xfs_dir2_dataptr_t, not a bare uint32_t. > > Me says: > The community can do what they want. If you want to start with that code, permission for S-O-B: me (snickers at the pun) > Thanks Mark. I guess we can add Mark's Signed-off-by to those patches he originally authored and that solves that problem. :) > > IMO, the best next step might be to just finish off the implementation > > as-is such that we could have a fairly functional RFC to put on the > > list and hash out whether there are in fact any remaining design > > hurdles, but others might have a different opinion on that. :) > > No one is asking me but I will add, IMO (definitely disclaiming the storage group and HPE): > the PIP costs outweigh the benefits. > Adding the redundant filename for recovery and an occasional output of the path pushes the cost to be *totally unreasonable*. > If I had a say, I don't and won't, I would NAK any implementation that included it. > Implementing with extended attributes is the wrong, wrong, wrong choice. It was done for hysterical :) reasons and even that appeasement bit me in the asterisk. > Could you elaborate on this a bit? I haven't gone through all of the old discussions and it's been a while since I've even reviewed the recent code, but on a quick look at the links Darrick posted, it appears the old implementation incorporated the "first" parent pointer direct in the inode with a fallback to xattrs for inodes with multiple parents whereas the updated implementation uses xattrs outright (and as a result is able to incorporate the dentry name in the xattr). Is that an accurate summary? If so, is the performance concern really the use of xattrs or the fact that the name is included (or some combination of both)? For example, would you consider a situation where pp's are entirely isolated to shortform xattrs much worse than the direct pp approach? It seems to me that the inline approach could still be considered as an optimization (i.e., maybe an incore cache) if performance does prove to be a problem. Maybe we could also consider something where (somehow, handwaving again ;) some number of pp xattrs are always shortform and thus remain within the inode. IOW, I think performance is a valid concern, but this should probably 1.) follow correctness and 2.) be more dictated by testing (another really good reason to try and finish off the current rfc IMO) where we can consider various, potentially problematic situations. We're bound to have workloads where pretty much every file is hardlinked more than once (think glusterfs), for example. Brian > Good luck. > > --Mark Tinguely > not representing anyone but myself. > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
