linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Davidlohr Bueso <dave@stgolabs.net>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	David Rientjes <rientjes@google.com>,
	Hugh Dickins <hughd@google.com>,
	Aneesh Kumar <aneesh.kumar@linux.vnet.ibm.com>,
	Hillf Danton <hillf.zj@alibaba-inc.com>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: [RFC v4 PATCH 2/9] mm/hugetlb: expose hugetlb fault mutex for use by fallocate
Date: Thu, 11 Jun 2015 16:09:58 -0700	[thread overview]
Message-ID: <557A1546.3090300@oracle.com> (raw)
In-Reply-To: <1434062766.3165.103.camel@stgolabs.net>

On 06/11/2015 03:46 PM, Davidlohr Bueso wrote:
> On Thu, 2015-06-11 at 14:01 -0700, Mike Kravetz wrote:
>>   /* Forward declaration */
>>   static int hugetlb_acct_memory(struct hstate *h, long delta);
>> @@ -3324,7 +3324,8 @@ static u32 fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
>>   	unsigned long key[2];
>>   	u32 hash;
>>
>> -	if (vma->vm_flags & VM_SHARED) {
>> +	/* !vma implies this was called from hugetlbfs fallocate code */
>> +	if (!vma || vma->vm_flags & VM_SHARED) {
>
> That !vma is icky, and really no need for it: hugetlbfs_fallocate(), for
> example, already passes [pseudo]vma->vm_flags with VM_SHARED, and you
> say it yourself in the comment. Do you see any reason why we cannot just
> keep the vma->vm_flags & VM_SHARED check?
>
>> +/*
>> + * Interface for use by hugetlbfs fallocate code.  Faults must be
>> + * synchronized with page adds or deletes by fallocate.  fallocate
>> + * only deals with shared mappings.  See also hugetlb_fault_mutex_lock
>> + * and hugetlb_fault_mutex_unlock.
>> + */
>> +u32 hugetlb_fault_mutex_shared_hash(struct address_space *mapping, pgoff_t idx)
>> +{
>> +	return fault_mutex_hash(NULL, NULL, NULL, mapping, idx, 0);
>> +}
>
> It strikes me that this too should be static inlined. But I really
> dislike the nil params thing, which should be addressed by my comment
> above.

In the previous RFC, I was trying not to make all the fault mutex data
global (so it could be accessed outside hugetlb.c).  That was the
original reason for the wrapper interfaces.  That may just be too ugly,
and does not buy us much.

Now that the mutex table is global for inlining, I might as well make
fault_mutex_hash() global.  I can then get rid of the wrappers.  However,
I'm guessing it would be a good idea to change the name(s) to something
hugetlb specific since they will be global.

-- 
Mike Kravetz

>
> Thanks,
> Davidlohr
>

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2015-06-11 23:10 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-11 21:01 [RFC v4 PATCH 0/9] hugetlbfs: add fallocate support Mike Kravetz
2015-06-11 21:01 ` [RFC v4 PATCH 1/9] mm/hugetlb: add region_del() to delete a specific range of entries Mike Kravetz
2015-06-11 21:01 ` [RFC v4 PATCH 2/9] mm/hugetlb: expose hugetlb fault mutex for use by fallocate Mike Kravetz
2015-06-11 22:46   ` Davidlohr Bueso
2015-06-11 23:09     ` Mike Kravetz [this message]
2015-06-17 22:05     ` Mike Kravetz
2015-06-11 21:01 ` [RFC v4 PATCH 3/9] hugetlbfs: hugetlb_vmtruncate_list() needs to take a range to delete Mike Kravetz
2015-06-11 21:01 ` [RFC v4 PATCH 4/9] hugetlbfs: truncate_hugepages() takes a range of pages Mike Kravetz
2015-06-11 21:01 ` [RFC v4 PATCH 5/9] mm/hugetlb: vma_has_reserves() needs to handle fallocate hole punch Mike Kravetz
2015-06-11 21:01 ` [RFC v4 PATCH 6/9] mm/hugetlb: alloc_huge_page handle areas hole punched by fallocate Mike Kravetz
2015-06-15  6:34   ` Naoya Horiguchi
2015-06-15 18:42     ` Mike Kravetz
2015-06-11 21:01 ` [RFC v4 PATCH 7/9] hugetlbfs: New huge_add_to_page_cache helper routine Mike Kravetz
2015-06-11 21:01 ` [RFC v4 PATCH 8/9] hugetlbfs: add hugetlbfs_fallocate() Mike Kravetz
2015-06-11 21:01 ` [RFC v4 PATCH 9/9] mm: madvise allow remove operation for hugetlbfs Mike Kravetz

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=557A1546.3090300@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dave@stgolabs.net \
    --cc=hch@infradead.org \
    --cc=hillf.zj@alibaba-inc.com \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=rientjes@google.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;
as well as URLs for NNTP newsgroup(s).