linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Konstantin Khlebnikov <khlebnikov@openvz.org>
To: sagig <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>, Oren Duer <oren@mellanox.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: Re: [PATCH RFC V2] mm: convert rcu_read_lock() to srcu_read_lock(), thus allowing to sleep in callbacks
Date: Wed, 08 Feb 2012 15:14:30 +0400	[thread overview]
Message-ID: <4F325916.20901@openvz.org> (raw)
In-Reply-To: <4F325587.4070605@mellanox.com>

sagig wrote:
> Hey Konstantin,
> Do you have any comments on V2?

only one, below.

>
> On 2/6/2012 4:40 PM, sagig@mellanox.com wrote:
>> From: Sagi Grimberg<sagig@mellanox.co.il>
>>
>> Now that anon_vma lock and i_mmap_mutex are both sleepable mutex, it is possible to schedule inside invalidation callbacks
>> (such as invalidate_page, invalidate_range_start/end and change_pte) .
>> This is essential for a scheduling HW sync in RDMA drivers which apply on demand paging methods.
>>
>> Signed-off-by: Sagi Grimberg<sagig@mellanox.co.il>
>> ---
>>    changes from V1:
>>    1. converted synchronize_rcu() to synchronize_srcu() to racing candidates (unregister, release)
>>    2. do_mmu_notifier_register: moved init_srcu_struct out of mmap_sem locking.
>>    3. do_mmu_notifier_register: fixed error path.
>>
>>    include/linux/mmu_notifier.h |    3 +++
>>    mm/mmu_notifier.c            |   38 +++++++++++++++++++++++++-------------
>>    2 files changed, 28 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
>> index 1d1b1e1..f3d6f30 100644
>> --- a/include/linux/mmu_notifier.h
>> +++ b/include/linux/mmu_notifier.h

>> @@ -226,8 +233,12 @@ static int do_mmu_notifier_register(struct mmu_notifier *mn,
>>    out_cleanup:
>>    	if (take_mmap_sem)
>>    		up_write(&mm->mmap_sem);
>> -	/* kfree() does nothing if mmu_notifier_mm is NULL */
>> -	kfree(mmu_notifier_mm);
>> +
>> +	if (mm->mmu_notifier_mm) {
>> +		/* in the case srcu_init failed srcu_cleanup is safe */
>> +		cleanup_srcu_struct(&mm->mmu_notifier_mm->srcu);
>> +		kfree(mmu_notifier_mm);
>> +	}

this seems incorrect. there must be:

if (mmu_notifier_mm) {
	cleanup_srcu_struct(&mmu_notifier_mm->srcu);
	kfree(mmu_notifier_mm);
}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

      reply	other threads:[~2012-02-08 11:14 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-06 14:40 sagig
2012-02-08 10:59 ` sagig
2012-02-08 11:14   ` Konstantin Khlebnikov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F325916.20901@openvz.org \
    --to=khlebnikov@openvz.org \
    --cc=linux-mm@kvack.org \
    --cc=ogerlitz@mellanox.com \
    --cc=oren@mellanox.com \
    --cc=sagig@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox