patch review scheduling...

patch review scheduling...

[Darrick J. Wong]

Hello list, frequent-submitters, and usual-reviewer-suspects:

As you've all seen, we have quite a backlog of patch review for 5.14
already.  The people cc'd on this message are the ones who either (a)
authored the patches caught in the backlog, (b) commented on previous
iterations of them, or (c) have participated in a lot of reviews before.

Normally I'd just chug through them all until I get to the end, but even
after speed-reading through the shorter series (deferred xattrs,
mmaplock, reflink+dax) I still have 73 to go, which is down from 109
this morning.

So, time to be a bit more aggressive about planning.  I would love it if
people dedicated some time this week to reviewing things, but before we
even get there, I have other questions:

Dave: Between the CIL improvements and the perag refactoring, which
would you rather get reviewed first?  The CIL improvments patches have
been circulating longer, but they're more subtle changes.

Dave and Christoph: Can I rely on you both to sort out whatever
conflicts arose around reworking memory page allocation for xfs_bufs?

Brian: Is it worth the time to iterate more on the ioend thresholds in
the "iomap: avoid soft lockup warnings" series?  Specifically, I was
kind of interested in whether or not we should/could scale the max ioend
size limit with the optimal/max io request size that devices report,
though I'm getting a feeling that block device limits are all over the
place maybe we should start with the static limit and iterate up (or
down?) from there...?

Everyone else: If you'd like to review something, please stake a claim
and start reading.

Everyone else not on cc: You're included too!  If you like! :)

--D

Re: patch review scheduling...

[Dave Chinner]

On Tue, May 25, 2021 at 06:27:04PM -0700, Darrick J. Wong wrote:
> Hello list, frequent-submitters, and usual-reviewer-suspects:
> 
> As you've all seen, we have quite a backlog of patch review for 5.14
> already.  The people cc'd on this message are the ones who either (a)
> authored the patches caught in the backlog, (b) commented on previous
> iterations of them, or (c) have participated in a lot of reviews before.
> 
> Normally I'd just chug through them all until I get to the end, but even
> after speed-reading through the shorter series (deferred xattrs,
> mmaplock, reflink+dax) I still have 73 to go, which is down from 109
> this morning.
> 
> So, time to be a bit more aggressive about planning.  I would love it if
> people dedicated some time this week to reviewing things, but before we
> even get there, I have other questions:
> 
> Dave: Between the CIL improvements and the perag refactoring, which
> would you rather get reviewed first?  The CIL improvments patches have
> been circulating longer, but they're more subtle changes.

The perag refactoring is already mostly reviewed and the changes are
simpler, so knock the rest of them out first.

The CIL series already has all the seperable changes up to patch 11
reviewed, so they can be merged without further review work.

The next chunk in that patch set is up to patch 25 (or is it 26?),
but I think there's only 4-5 patches in that set that are not yet
reviewed. That would be the next set to look at.

The rest of the patchset is the scalabilty stuff which probably is
going to be too much at this point, so we can leave it off to the
next cycle (when I'll put out another 20-30 patches I already have
on top of this with it...)

> Dave and Christoph: Can I rely on you both to sort out whatever
> conflicts arose around reworking memory page allocation for xfs_bufs?

Yep, we'll get that sorted out. There's no disagreement on where we
want to end up, just on how we get there :P

> Brian: Is it worth the time to iterate more on the ioend thresholds in
> the "iomap: avoid soft lockup warnings" series?  Specifically, I was
> kind of interested in whether or not we should/could scale the max ioend
> size limit with the optimal/max io request size that devices report,
> though I'm getting a feeling that block device limits are all over the
> place maybe we should start with the static limit and iterate up (or
> down?) from there...?

I recommend just staying with static limits to start with. This is
"good enough" to solve the immediate problems, and we can look for
more optimised solutions once we've merged the initial fixes...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: patch review scheduling...

[Brian Foster]

On Tue, May 25, 2021 at 06:27:04PM -0700, Darrick J. Wong wrote:
> Hello list, frequent-submitters, and usual-reviewer-suspects:
> 
> As you've all seen, we have quite a backlog of patch review for 5.14
> already.  The people cc'd on this message are the ones who either (a)
> authored the patches caught in the backlog, (b) commented on previous
> iterations of them, or (c) have participated in a lot of reviews before.
> 
> Normally I'd just chug through them all until I get to the end, but even
> after speed-reading through the shorter series (deferred xattrs,
> mmaplock, reflink+dax) I still have 73 to go, which is down from 109
> this morning.
> 
> So, time to be a bit more aggressive about planning.  I would love it if
> people dedicated some time this week to reviewing things, but before we
> even get there, I have other questions:
> 
> Dave: Between the CIL improvements and the perag refactoring, which
> would you rather get reviewed first?  The CIL improvments patches have
> been circulating longer, but they're more subtle changes.
> 
> Dave and Christoph: Can I rely on you both to sort out whatever
> conflicts arose around reworking memory page allocation for xfs_bufs?
> 
> Brian: Is it worth the time to iterate more on the ioend thresholds in
> the "iomap: avoid soft lockup warnings" series?  Specifically, I was
> kind of interested in whether or not we should/could scale the max ioend
> size limit with the optimal/max io request size that devices report,
> though I'm getting a feeling that block device limits are all over the
> place maybe we should start with the static limit and iterate up (or
> down?) from there...?
> 

I was starting to think about the optimal I/O size thing yesterday given
the latest feedback. I think it makes sense and it's probably easy
enough to incorporate, but if you're asking me about patch processing
logistics, IMO none of the changes or outstanding feedback since the v2
(inline w/ v1) are terribly important to fix the original problem.

Most of the feedback since v2 has been additive (i.e. "fix this problem
too") or surmising about inconsequential things like cond_resched()
usage or whether the threshold should be defined based on pages or not.
v2 used a large threshold to avoid things like risk of
unintended/unexpected consequences causing a revert down the line and
reintroducing the soft lockup problem, which is otherwise easily fixable
without significant change to functional behavior (given the current
worst case of unbound aggregation). So since you ask and after having
thought about it, if you're looking for a targeted fix to merge sooner
rather than later I think the smart thing to do is stick with v2 and
rebase the subsequent changes to reduce interrupt context latency and
general completion latency on top of that. (In fact, I probably should
have done that for v3.)

Brian

> Everyone else: If you'd like to review something, please stake a claim
> and start reading.
> 
> Everyone else not on cc: You're included too!  If you like! :)
> 
> --D
> 

Re: patch review scheduling...

[Darrick J. Wong]

On Wed, May 26, 2021 at 07:53:11AM -0400, Brian Foster wrote:
> On Tue, May 25, 2021 at 06:27:04PM -0700, Darrick J. Wong wrote:
> > Hello list, frequent-submitters, and usual-reviewer-suspects:
> > 
> > As you've all seen, we have quite a backlog of patch review for 5.14
> > already.  The people cc'd on this message are the ones who either (a)
> > authored the patches caught in the backlog, (b) commented on previous
> > iterations of them, or (c) have participated in a lot of reviews before.
> > 
> > Normally I'd just chug through them all until I get to the end, but even
> > after speed-reading through the shorter series (deferred xattrs,
> > mmaplock, reflink+dax) I still have 73 to go, which is down from 109
> > this morning.
> > 
> > So, time to be a bit more aggressive about planning.  I would love it if
> > people dedicated some time this week to reviewing things, but before we
> > even get there, I have other questions:
> > 
> > Dave: Between the CIL improvements and the perag refactoring, which
> > would you rather get reviewed first?  The CIL improvments patches have
> > been circulating longer, but they're more subtle changes.
> > 
> > Dave and Christoph: Can I rely on you both to sort out whatever
> > conflicts arose around reworking memory page allocation for xfs_bufs?
> > 
> > Brian: Is it worth the time to iterate more on the ioend thresholds in
> > the "iomap: avoid soft lockup warnings" series?  Specifically, I was
> > kind of interested in whether or not we should/could scale the max ioend
> > size limit with the optimal/max io request size that devices report,
> > though I'm getting a feeling that block device limits are all over the
> > place maybe we should start with the static limit and iterate up (or
> > down?) from there...?
> > 
> 
> I was starting to think about the optimal I/O size thing yesterday given
> the latest feedback. I think it makes sense and it's probably easy
> enough to incorporate, but if you're asking me about patch processing
> logistics, IMO none of the changes or outstanding feedback since the v2
> (inline w/ v1) are terribly important to fix the original problem.
> 
> Most of the feedback since v2 has been additive (i.e. "fix this problem
> too") or surmising about inconsequential things like cond_resched()
> usage or whether the threshold should be defined based on pages or not.
> v2 used a large threshold to avoid things like risk of
> unintended/unexpected consequences causing a revert down the line and
> reintroducing the soft lockup problem, which is otherwise easily fixable
> without significant change to functional behavior (given the current
> worst case of unbound aggregation). So since you ask and after having
> thought about it, if you're looking for a targeted fix to merge sooner
> rather than later I think the smart thing to do is stick with v2 and
> rebase the subsequent changes to reduce interrupt context latency and
> general completion latency on top of that. (In fact, I probably should
> have done that for v3.)

Yeah, this basically comes down to: take v2 as a fix for 5.13?  Or v3
as a larger fix for 5.14?  I guess I'm the one ranting about having too
many stall warning escalations, so it's up to me to pick something.
TBH I like the "put v3 in 5.14" option since it gives us a much longer
testing ramp...

--D

> 
> Brian
> 
> > Everyone else: If you'd like to review something, please stake a claim
> > and start reading.
> > 
> > Everyone else not on cc: You're included too!  If you like! :)
> > 
> > --D
> > 
>