linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	overlayfs <linux-unionfs@vger.kernel.org>,
	linux-kernel@vger.kernel.org, Miklos Szeredi <miklos@szeredi.hu>,
	Matthew Wilcox <willy@infradead.org>,
	Colin Walters <walters@verbum.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	syzbot <syzbot+d6ec23007e951dadf3de@syzkaller.appspotmail.com>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>
Subject: Re: [PATCH v4 1/2] hugetlb: use f_mode & FMODE_HUGETLBFS to identify hugetlbfs files
Date: Fri, 12 Jun 2020 14:51:54 -0700	[thread overview]
Message-ID: <b1756da5-4e91-298f-32f1-e5642a680cbf@oracle.com> (raw)
In-Reply-To: <20200612015842.GC23230@ZenIV.linux.org.uk>

On 6/11/20 6:58 PM, Al Viro wrote:
> On Thu, Jun 11, 2020 at 05:46:43PM -0700, Mike Kravetz wrote:
>> The routine is_file_hugepages() checks f_op == hugetlbfs_file_operations
>> to determine if the file resides in hugetlbfs.  This is problematic when
>> the file is on a union or overlay.  Instead, define a new file mode
>> FMODE_HUGETLBFS which is set when a hugetlbfs file is opened.  The mode
>> can easily be copied to other 'files' derived from the original hugetlbfs
>> file.
>>
>> With this change hugetlbfs_file_operations can be static as it should be.
>>
>> There is also a (duplicate) set of shm file operations used for the routine
>> is_file_shm_hugepages().  Instead of setting/using special f_op's, just
>> propagate the FMODE_HUGETLBFS mode.  This means is_file_shm_hugepages() and
>> the duplicate f_ops can be removed.
> 
> s/HUGETLBFS/HUGEPAGES/, please.
> 
>> While cleaning things up, change the name of is_file_hugepages() to
>> is_file_hugetlbfs().  The term hugepages is a bit ambiguous.
> 
> Don't, especially since the very next patch adds such on overlayfs...

Ok. This is just something I thought might clarify things.  I seem to
recall questions about 'huge page' routines such as "is that for THP or
hugetlb huge pages"?  That was my motivation for the change.  Since this
is only about hugetlbfs, make it explicit.

> Incidentally, can a hugetlbfs be a lower layer, while the upper one
> is a normal filesystem?  What should happen on copyup?

Yes, that seems to work as expected.  When accessed for write the hugetlb
file is copied to the normal filesystem.

The BUG found by syzbot actually has a single hugetlbfs as both lower and
upper.  With the BUG 'fixed', I am not exactly sure what the expected
behavior is in this case.  I may be wrong, but I would expect any operations
that can be performed on a stand alone hugetlbfs to also be performed on
the overlay.  However, mmap() still fails.  I will look into it.

I also looked at normal filesystem lower and hugetlbfs upper.  Yes, overlayfs
allows this.  This is somewhat 'interesting' as write() is not supported in
hugetlbfs.  Writing to files in the overlay actually ended up writing to
files in the lower filesystem.  That seems wrong, but overlayfs is new to me.

Earlier in the discussion of these issues, Colin Walters asked "Is there any
actual valid use case for mounting an overlayfs on top of hugetlbfs?"  I can
not think of one.  Perhaps we should consider limiting the ways in which
hugetlbfs can be used in overlayfs?  Preventing it from being an upper
filesystem might be a good start?  Or, do people think making hugetlbfs and
overlayfs play nice together is useful?
-- 
Mike Kravetz

  reply	other threads:[~2020-06-12 21:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-12  0:46 [PATCH v4 1/2] hugetlb: use f_mode & FMODE_HUGETLBFS to identify hugetlbfs files Mike Kravetz
2020-06-12  0:46 ` [PATCH v4 2/2] ovl: call underlying get_unmapped_area() routine. propogate FMODE_HUGETLBFS Mike Kravetz
2020-06-14 12:50   ` Amir Goldstein
2020-06-12  1:53 ` [PATCH v4 1/2] hugetlb: use f_mode & FMODE_HUGETLBFS to identify hugetlbfs files Matthew Wilcox
2020-06-12  1:58 ` Al Viro
2020-06-12 21:51   ` Mike Kravetz [this message]
2020-06-13  6:53     ` Amir Goldstein
2020-06-13 14:38       ` Matthew Wilcox
2020-06-13 19:12       ` Mike Kravetz
2020-06-15  7:53         ` Miklos Szeredi
2020-06-15 10:05           ` Amir Goldstein
2020-06-15 13:01             ` Miklos Szeredi
2020-06-15 23:45           ` Mike Kravetz
2020-06-16  9:01             ` Miklos Szeredi
2020-06-15  8:24       ` Miklos Szeredi
2020-06-15 17:48         ` Mike Kravetz
2020-06-12  6:28 ` [RFC PATCH] hugetlb: hugetlbfs_file_operations can be static kernel test robot
2020-06-13 14:19 ` [hugetlb] 5156c6c1b9: INFO:trying_to_register_non-static_key kernel test robot

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=b1756da5-4e91-298f-32f1-e5642a680cbf@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=syzbot+d6ec23007e951dadf3de@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=walters@verbum.org \
    --cc=willy@infradead.org \
    /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).