Prev: [PATCH 1/4] fix kfifo miss use of nozami.c
Next: [PATCH 3/4] replace the old non generic API
From: Miao Xie on 12 May 2010 05:00
on 2010-5-12 12:32, Andrew Morton wrote:
> On Wed, 12 May 2010 15:20:51 +0800 Miao Xie <miaox(a)cn.fujitsu.com> wrote:
>
>> @@ -985,6 +984,7 @@ repeat:
>> * for the read-side.
>> */
>> while (ACCESS_ONCE(tsk->mems_allowed_change_disable)) {
>> + task_unlock(tsk);
>> if (!task_curr(tsk))
>> yield();
>> goto repeat;
>
> Oh, I meant to mention that. No yield()s, please. Their duration is
> highly unpredictable. Can we do something more deterministic here?

Maybe we can use wait_for_completion().

>
> Did you consider doing all this with locking? get_mems_allowed() does
> mutex_lock(current->lock)?

do you means using a real lock(such as: mutex) to protect mempolicy and mem_allowed?

It may cause the performance regression, so I do my best to abstain from using a real
lock.

Thanks
Miao

>
>
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Miao Xie on 13 May 2010 02:20
on 2010-5-13 1:48, Andrew Morton wrote:
>> It may cause the performance regression, so I do my best to abstain from using a real
>> lock.
>
> Well, the code as-is is pretty exotic with lots of open-coded tricky
> barriers - it's best to avoid inventing new primitives if possible.
> For example, there's no lockdep support for this new "lock".

I didn't find an existing lock that could fix the problem well till now, so
I had to design this new "lock" to protect the task's mempolicy and mems_allowed.

>
> mutex_lock() is pretty quick - basically a simgle atomic op. How
> frequently do these operations occur?

There is another problem that I forgot to mention.
besides the performance problem, the read-side may call it in the context
in which the task can't sleep. so we can't use mutex.

>
> The code you have at present is fairly similar to sequence locks. I
> wonder if there's some way of (ab)using sequence locks for this.
> seqlocks don't have lockdep support either...
>

We can't use sequence locks here, because the read-side may read the data
in changing, but it can't put off cleaning the old bits.

Thanks
Miao

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Miao Xie on 17 May 2010 00:10
on 2010-5-14 3:11, Andrew Morton wrote:
> On Thu, 13 May 2010 14:16:33 +0800
> Miao Xie <miaox(a)cn.fujitsu.com> wrote:
>
>>>
>>> The code you have at present is fairly similar to sequence locks. I
>>> wonder if there's some way of (ab)using sequence locks for this.
>>> seqlocks don't have lockdep support either...
>>>
>>
>> We can't use sequence locks here, because the read-side may read the data
>> in changing, but it can't put off cleaning the old bits.
>
> I don't understand that sentence. Can you expand on it please?
>

the mempolicy and mems_allowed tell the task that it should allocates the memory
space on the specified node. so when allocating the memory space, the memory
allocation functions that the task invokes must accesses the mempolicy and
mems_allowed to find a node on which it can do memory allocation.

But those memory allocation functions can be used in both the context that the
task can sleep and the context that the task can't sleep(etc. disable irq). so
the real lock is not suitable.

And it is not a problem that the task allocates the memory space on the old
allowed node when the mempolicy and mems_allowed is in changing, because the
mempolicy and mems_allowed is not mandatory. So I think we needn't use a real
lock to protect the mempolicy and mems_allowed in the read-side, and just use a
real lock in the write-side. But there is a serious problem, that is the read
-side may find no node to allocate memory and oom occurs, just like the
following case(mentioned in the patch's changelog):
(mpol: mempolicy)
task1 task1's mpol task2
alloc page 1
alloc on node0? NO 1
1 change mems from 1 to 0
0 rebind task1's mpol
alloc on node1? NO 0
...
can't alloc page
goto oom

In order to fix this problem, I got an idea that we set the newly allowed nodes
first, and then clean the disallowed nodes, But there is still a problem.
(mpol: mempolicy)
task1 task1's mpol task2
alloc page 1
alloc on node0? NO 1
1 change mems from 1 to 0
1 rebind task1's mpol
0-1 set new bits
0 clear disallowed bits
alloc on node1? NO 0
...
can't alloc page
goto oom

It is because we cleanup disallowed nodes early, so I use a variable to tell the
write-side that the task is accessing the mempolicy and mems_allowed now, the
write-side must cleanup disallowed nodes soon after.

And the seq read lock can't provide this function. And besides that, the read-side
will goto oom and not go back if it find no node to allcate memory, so it won't
check the seq number of lock to find whether the mempolicy and mems_allowed have
been changed. so the seq lock is also not suitable, I think.

Thanks
Miao

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: [PATCH 1/4] fix kfifo miss use of nozami.c
Next: [PATCH 3/4] replace the old non generic API