On Tue, 07 Sep 2021, Chuck Lever III wrote: > Hi Neil- > > > On Sep 6, 2021, at 12:44 AM, NeilBrown <neilb@xxxxxxx> wrote: > > > > > > Many places that need to wait before retrying a memory allocation use > > congestion_wait(). xfs_buf_alloc_pages() is a good example which > > follows a similar pattern to that in svc_alloc_args(). > > > > It make sense to do the same thing in svc_alloc_args(); This will allow > > the allocation to be retried sooner if some backing device becomes > > non-congested before the timeout. > > > > Every call to congestion_wait() in the entire kernel passes BLK_RW_ASYNC > > as the first argument, so we should so. > > > > The second argument - an upper limit for waiting - seem fairly > > arbitrary. Many places use "HZ/50" or "HZ/10". As there is no obvious > > choice, it seems reasonable to leave the maximum time unchanged. > > > > If a service using svc_alloc_args() is terminated, it may now have to > > wait up to the full 500ms before termination completes as > > congestion_wait() cannot be interrupted. I don't believe this will be a > > problem in practice, though it might be justification for using a > > smaller timeout. > > > > Signed-off-by: NeilBrown <neilb@xxxxxxx> > > --- > > > > I happened to notice this inconsistency between svc_alloc_args() and > > xfs_buf_alloc_pages() despite them doing very similar things, so thought > > I'd send a patch. > > > > NeilBrown > > When we first considered the alloc_pages_bulk() API, the SUNRPC > patch in that series replaced this schedule_timeout(). Mel > suggested we postpone that to a separate patch. Now is an ideal > time to consider this change again. I've added the MM folks for > expert commentary. > > I would rather see a shorter timeout, since that will be less > disruptive in practice and today's systems shouldn't have to wait > that long for free memory to become available. DEFAULT_IO_TIMEOUT > might be a defensible choice -- it will slow down this loop > effectively without adding a significant delay. DEFAULT_IO_TIMEOUT is local to f2fs, so might not be the best choice. I could be comfortable with any number from 1 to HZ, and have no basis on how to make a choice - which is why I deliberately avoided making one. Ideally, the full timeout would (almost) never expire in practice. Ideally, the interface would not even ask that we supply a timeout. But are not currently at the ideal ;-( Thanks, NeilBrown
