On 12/24/19 5:25 AM, Christoph Hellwig wrote:
On Wed, Dec 11, 2019 at 09:15:07PM -0700, Allison Collins wrote:
New delayed attribute routines cannot handle transactions. We
can factor up the commit, but there is little left in this
function other than some error handling and an ichgtime. So
hoist all of xfs_attr_try_sf_addname up at this time. We will
remove all the commits in this set.
I really don't like this one. xfs_attr_try_sf_addname is a nice
abstration, so merging it into the caller makes the code much harder
to understand. If that is different after changes to the transaction
change it can be removed at that point, but merging all the different
attr formats into one big monster function is a bad idea.
Well, in the last version of the set, I did keep it around, but it was
so small, Darrick suggested cleaning it out all together. Here is the
same patch in the last set:
https://patchwork.kernel.org/patch/11231511/
Maybe when more folks get back from break they can chime in about
keeping it or not. I think I commented pretty well on "monster
function" in the last patch. It is in all in the pursuit of pulling
things up and then breaking them back down again.
Also Factor up still sounds very, very strange to me. I would
have worded it as "merge xfs_attr_try_sf_addname into its only caller"
I always assumed it to be a figure of speech in reference to factoring
out common code, sort of like how one might factor out a common
denominator or common multiple from an algebraic expression. If it
really bothers folks though, I don't mind changing it.
Allison
