All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>, Linux MM <linux-mm@kvack.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	overlayfs <linux-unionfs@vger.kernel.org>,
	linux-kernel <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: Sat, 13 Jun 2020 12:12:04 -0700	[thread overview]
Message-ID: <6e8924b0-bfc4-eaf5-1775-54f506cdf623@oracle.com> (raw)
In-Reply-To: <CAOQ4uxg=o2SVbfUiz0nOg-XHG8irvAsnXzFWjExjubk2v_6c_A@mail.gmail.com>

On 6/12/20 11:53 PM, Amir Goldstein wrote:
>>> 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.
>>
> 
> I am not sure how that happened, but I think that ovl_open_realfile()
> needs to fixup f_mode flags FMODE_CAN_WRITE | FMODE_CAN_READ
> after open_with_fake_path().
> 
>> 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?
> 
> If people think that making hugetlbfs and overlayfs play nice together maybe
> they should work on this problem. It doesn't look like either
> hugetlbfs developers
> nor overlayfs developers care much about the combination.

Thanks Amir,

As a hugetlbfs developer, I do not know of a use case for interoperability
with overlayfs.  So yes, I am not too interested in making them work well
together.  However, if there was an actual use case I would be more than
happy to consider doing the work.  Just hate to put effort into fixing up
two 'special' filesystems for functionality that may not be used.

I can't speak for overlayfs developers.

> Your concern, I assume, is fixing the syzbot issue.

That is the primary concern.  We should not BUG!  After fixing that up, Al
asked how these things worked together.  I honestly did not look at
interoperability before that.  I am not sure if anyone has done that in the
past.

> I agree with Colin's remark about adding limitations, but it would be a shame
> if overlay had to special case hugetlbfs. It would have been better if we could
> find a property of hugetlbfs that makes it inapplicable for overlayfs
> upper/lower
> or stacking fs in general.
> 
> The simplest thing for you to do in order to shush syzbot is what procfs does:
>         /*
>          * procfs isn't actually a stacking filesystem; however, there is
>          * too much magic going on inside it to permit stacking things on
>          * top of it
>          */
>         s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
> 
> Currently, the only in-tree stacking fs are overlayfs and ecryptfs, but there
> are some out of tree implementations as well (shiftfs).
> So you may only take that option if you do not care about the combination
> of hugetlbfs with any of the above.
> 
> overlayfs support of mmap is not as good as one might hope.
> overlayfs.rst says:
> "If a file residing on a lower layer is opened for read-only and then
>  memory mapped with MAP_SHARED, then subsequent changes to
>  the file are not reflected in the memory mapping."
> 
> So if I were you, I wouldn't go trying to fix overlayfs-huguetlb interop...

Thanks again,

I'll look at something as simple as s_stack_depth.
-- 
Mike Kravetz

  parent reply	other threads:[~2020-06-13 19:12 UTC|newest]

Thread overview: 27+ 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-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
2020-06-13  6:53     ` Amir Goldstein
2020-06-13  6:53       ` Amir Goldstein
2020-06-13 14:38       ` Matthew Wilcox
2020-06-13 19:12       ` Mike Kravetz [this message]
2020-06-15  7:53         ` Miklos Szeredi
2020-06-15  7:53           ` Miklos Szeredi
2020-06-15 10:05           ` Amir Goldstein
2020-06-15 10:05             ` Amir Goldstein
2020-06-15 13:01             ` Miklos Szeredi
2020-06-15 13:01               ` Miklos Szeredi
2020-06-15 23:45           ` Mike Kravetz
2020-06-16  9:01             ` Miklos Szeredi
2020-06-16  9:01               ` Miklos Szeredi
2020-06-15  8:24       ` 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-12  6:28   ` kernel test robot
2020-06-13 14:19 ` [hugetlb] 5156c6c1b9: INFO:trying_to_register_non-static_key kernel test robot
2020-06-13 14:19   ` 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=6e8924b0-bfc4-eaf5-1775-54f506cdf623@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=amir73il@gmail.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.