On 1/25/20 4:08 PM, Christoph Hellwig wrote:
On Sat, Jan 25, 2020 at 09:27:39AM -0700, Allison Collins wrote:
I was thinking of adapting this patch to be part of a 3 patch series
including one of Chistophs. Something like this:
[PATCH v6 11/16] xfs: Check for -ENOATTR or -EEXIST
[PATCH v6 03/16] xfs: Add xfs_has_attr and subroutines
[PATCH 02/29] xfs: merge xfs_attr_remove into xfs_attr_set
What would people think of that? I figured this would be better than the
two of us bombarding the mailing list with giant conflicting sets? Also, was
I able to answer everyones questions on this patch?
I'm still not sold at all on this series. It adds a lot of code and
makes it much harder to understand. So I'd much rather go back and
figuring out how we can do delayed attrs in a more streamlined way.
I had something like that quite a long time ago. Instead of the EAGAIN
ping pong, we had a boolean that just sort of turned the transactions
off in the delayed attrs path:
https://www.spinics.net/lists/linux-xfs/msg18114.html
The trouble with this is that it creates one large transaction rather
than breaking them up. So while the new method is a lot more complex,
it seemed to be the direction folks preferred.
The has_attr and co changes are some of exactly that kind of logic
that is just making things worse in the standalone patch set, so even
if we must end up with it they absolutely belong into a series actually
adding functionality, as they have no use on their own. >
Well, in an earlier discussion though we had come together with a plan
of implementing the new feature through a series of sub sets. First of
which being pure refactor with no functional change.
https://www.spinics.net/lists/linux-xfs/msg32035.html
I do try to keep the extended set up to date though. Mostly for my own
sanity and to make sure things are still on track for the long term
goals we are aiming for.
Independent of that we'll need to clean up the flags mess, so I'd rather
just go ahead with that for now.
