xfs
[Top] [All Lists]

Re: [PATCH] xfs: pull up stack_switch check into xfs_bmapi_write

To: Mark Tinguely <tinguely@xxxxxxx>
Subject: Re: [PATCH] xfs: pull up stack_switch check into xfs_bmapi_write
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Thu, 17 Jan 2013 15:49:52 -0500
Cc: xfs@xxxxxxxxxxx, dchinner@xxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <50F8480F.4090201@xxxxxxx>
References: <1358446289-871-1-git-send-email-bfoster@xxxxxxxxxx> <50F8480F.4090201@xxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0
On 01/17/2013 01:50 PM, Mark Tinguely wrote:
> On 01/17/13 12:11, Brian Foster wrote:
>> The stack_switch check currently occurs in __xfs_bmapi_allocate,
>> which means the stack switch only occurs when xfs_bmapi_allocate()
>> is called in a loop. Pull the check up before the loop in
>> xfs_bmapi_write() such that the first iteration of the loop has
>> consistent behavior.
>>
>> Signed-off-by: Brian Foster<bfoster@xxxxxxxxxx>
>> ---
>>
>> I was reading through this code and confused myself over whether the
>> stack
>> switch ever actually occurs. Eric and Ben pointed out on irc
>> (simultaneously,
>> I might add) the surrounding loop that I had missed, but it wasn't
>> clear whether
>> the behavior to enable the stack switch after the first iteration was
>> intentional or not. I'm throwing this out there to either fix the
>> issue or seek
>> out an explanation for the existing behavior. Thanks!
>>
>> Brian
>>
...
> 
> 
> Might want to read the Sep 2012 OSS archives, for example the below
> thread and the spin off threads.
> 
>  http://oss.sgi.com/archives/xfs/2012-09/msg00254.html
> 

Thanks for the pointer Mark. I think I follow the gist of the original
problem based on your example (e.g., flusher kicks in and gets a perag,
a separate xfsalloc worker is started and pends on the perag, meanwhile
it has consumed the worker resource that the original flusher ultimately
needed to proceed).

> My original patch was in xfs_bmapi_write() and it was moved to its
> current location.
> 

So that points me to Dave's fix:

http://oss.sgi.com/archives/xfs/2012-10/msg00080.html

... where he calls out doing the stack switch from a limited context
(via the new flag) and then moving it into xfs_bmapi_allocate(). I can't
say I completely understand the solution here, but my specific question
and reason for this patch is to understand that for the case where we do
switch stacks (and pass XFS_BMAPI_STACK_SWITCH), is it intentional or
part of the solution that we don't actually do the switch until the
second (and subsequent) iteration of the loop in xfs_bmapi_write()?

Brian

> You can force the code by restricting to only one allocation worker and
> there were 2 xfstests (76? and 139? come to mind) that quickly triggered
> the event.
> 
> --Mark.

<Prev in Thread] Current Thread [Next in Thread>