* Re: [PATCH 1/1] dm-zoned: Zoned block device target
From: Damien Le Moal @ 2017-02-10 0:51 UTC (permalink / raw)
To: Christoph Hellwig
Cc: dm-devel, Alasdair Kergon, Mike Snitzer, Hannes Reinecke,
linux-block
In-Reply-To: <20170209093606.GA8189@lst.de>
Christoph,
On 2/9/17 18:36, Christoph Hellwig wrote:
> On Thu, Feb 09, 2017 at 01:18:49PM +0900, Damien Le Moal wrote:
>> +
>> +/*
>> + * Target BIO completion.
>> + */
>> +static inline void dmz_bio_end(struct bio *bio, int err)
>> +{
>> + struct dm_zone_bioctx *bioctx =
>> + dm_per_bio_data(bio, sizeof(struct dm_zone_bioctx));
>> +
>> + if (atomic_dec_and_test(&bioctx->ref)) {
>
> It seems like this could be simplified a bit by using bio_chain
> to chain the clones to the original bio.
Thank you for the hint. I will look into this.
Best regards.
--
Damien Le Moal, Ph.D.
Sr. Manager, System Software Research Group,
Western Digital Corporation
Damien.LeMoal@wdc.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com
^ permalink raw reply
* Re: [PATCH 1/1] dm-zoned: Zoned block device target
From: Damien Le Moal @ 2017-02-10 1:09 UTC (permalink / raw)
To: Mike Snitzer
Cc: dm-devel, Alasdair Kergon, Hannes Reinecke, Christoph Hellwig,
linux-block, Bart Van Assche
In-Reply-To: <20170209152745.GA10638@redhat.com>
Mike,
On 2/10/17 00:27, Mike Snitzer wrote:
> Looks like this work has come a long way. While I am _still_ not
> hearing from any customers (or partners) that SMR is a priority I do
> acknowledge that that obviously isn't the case for WDC and others.
>
> So, I'll sign up for reviewing this DM target, with a focus on DM
> interface and implementation details, _after_ I get some technical
> review (e.g. Reviewed-by) from people like Hannes and others more
> invested in the SMR technologies. Basically I don't know enough about
> the constraints of these devices to fully appreciate and catch some
> issue in the particulars of the algorithms used to mitigate SMR's
> quirks.
Fair enough. I understand.
I would also like to let you know that I worked closely with Hannes and
Martin and also Shaun Tauncheff (Seagate) to put together the final
patches of the ZBC support for 4.10. I also contributed to part of the
f2fs support for these devices, which also was included in 4.10.
(And I also maintain the libzbc project in github)
> I'd also like to understand how invested WDC is in maintaining this DM
> target for years and years if/when I were to merge this DM target into
> mainline.
We are fully invested. I am part of the system software group in WD
Research and one of our mission is to support Linux kernel and open
source software in general. Bart (Van Assche), who I believe you know,
is also a member of our group now that Sandisk and WD merger completed.
Hannes and Christoph already sent some comments regarding the code. I
will make appropriate changes and re-post the patch for review.
Thank you.
Best regards.
--
Damien Le Moal, Ph.D.
Sr. Manager, System Software Research Group,
Western Digital Corporation
Damien.LeMoal@wdc.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com
^ permalink raw reply
* Re: [PATCHv6 11/37] HACK: readahead: alloc huge pages, if allowed
From: Andreas Dilger @ 2017-02-10 0:23 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kirill A. Shutemov, Theodore Ts'o, Andreas Dilger, Jan Kara,
Andrew Morton, Alexander Viro, Hugh Dickins, Andrea Arcangeli,
Dave Hansen, Vlastimil Babka, Ross Zwisler, linux-ext4,
linux-fsdevel, linux-kernel, linux-mm, linux-block
In-Reply-To: <20170209233436.GZ2267@bombadil.infradead.org>
[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]
On Feb 9, 2017, at 4:34 PM, Matthew Wilcox <willy@infradead.org> wrote:
>
> On Thu, Jan 26, 2017 at 02:57:53PM +0300, Kirill A. Shutemov wrote:
>> Most page cache allocation happens via readahead (sync or async), so if
>> we want to have significant number of huge pages in page cache we need
>> to find a ways to allocate them from readahead.
>>
>> Unfortunately, huge pages doesn't fit into current readahead design:
>> 128 max readahead window, assumption on page size, PageReadahead() to
>> track hit/miss.
>>
>> I haven't found a ways to get it right yet.
>>
>> This patch just allocates huge page if allowed, but doesn't really
>> provide any readahead if huge page is allocated. We read out 2M a time
>> and I would expect spikes in latancy without readahead.
>>
>> Therefore HACK.
>>
>> Having that said, I don't think it should prevent huge page support to
>> be applied. Future will show if lacking readahead is a big deal with
>> huge pages in page cache.
>>
>> Any suggestions are welcome.
>
> Well ... what if we made readahead 2 hugepages in size for inodes which
> are using huge pages? That's only 8x our current readahead window, and
> if you're asking for hugepages, you're accepting that IOs are going to
> be larger, and you probably have the kind of storage system which can
> handle doing larger IOs.
It would be nice if the bdi had a parameter for the maximum readahead size.
Currently, readahead is capped at 2MB chunks by force_page_cache_readahead()
even if bdi->ra_pages and bdi->io_pages are much larger.
It should be up to the filesystem to decide how large the readahead chunks
are rather than imposing some policy in the MM code. For high-speed (network)
storage access it is better to have at least 4MB read chunks, for RAID storage
it is desirable to have stripe-aligned readahead to avoid read inflation when
verifying the parity. Any fixed size will eventually be inadequate as disks
and filesystems change, so it may as well be a per-bdi tunable that can be set
by the filesystem as needed, or possibly with a mount option if needed.
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: [PATCHv6 11/37] HACK: readahead: alloc huge pages, if allowed
From: Matthew Wilcox @ 2017-02-09 23:34 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Andrew Morton,
Alexander Viro, Hugh Dickins, Andrea Arcangeli, Dave Hansen,
Vlastimil Babka, Ross Zwisler, linux-ext4, linux-fsdevel,
linux-kernel, linux-mm, linux-block
In-Reply-To: <20170126115819.58875-12-kirill.shutemov@linux.intel.com>
On Thu, Jan 26, 2017 at 02:57:53PM +0300, Kirill A. Shutemov wrote:
> Most page cache allocation happens via readahead (sync or async), so if
> we want to have significant number of huge pages in page cache we need
> to find a ways to allocate them from readahead.
>
> Unfortunately, huge pages doesn't fit into current readahead design:
> 128 max readahead window, assumption on page size, PageReadahead() to
> track hit/miss.
>
> I haven't found a ways to get it right yet.
>
> This patch just allocates huge page if allowed, but doesn't really
> provide any readahead if huge page is allocated. We read out 2M a time
> and I would expect spikes in latancy without readahead.
>
> Therefore HACK.
>
> Having that said, I don't think it should prevent huge page support to
> be applied. Future will show if lacking readahead is a big deal with
> huge pages in page cache.
>
> Any suggestions are welcome.
Well ... what if we made readahead 2 hugepages in size for inodes which
are using huge pages? That's only 8x our current readahead window, and
if you're asking for hugepages, you're accepting that IOs are going to
be larger, and you probably have the kind of storage system which can
handle doing larger IOs.
^ permalink raw reply
* Re: [PATCHv6 10/37] filemap: handle huge pages in filemap_fdatawait_range()
From: Matthew Wilcox @ 2017-02-09 23:03 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Andrew Morton,
Alexander Viro, Hugh Dickins, Andrea Arcangeli, Dave Hansen,
Vlastimil Babka, Ross Zwisler, linux-ext4, linux-fsdevel,
linux-kernel, linux-mm, linux-block
In-Reply-To: <20170126115819.58875-11-kirill.shutemov@linux.intel.com>
On Thu, Jan 26, 2017 at 02:57:52PM +0300, Kirill A. Shutemov wrote:
> @@ -405,9 +405,14 @@ static int __filemap_fdatawait_range(struct address_space *mapping,
> if (page->index > end)
> continue;
>
> + page = compound_head(page);
> wait_on_page_writeback(page);
> if (TestClearPageError(page))
> ret = -EIO;
> + if (PageTransHuge(page)) {
> + index = page->index + HPAGE_PMD_NR;
> + i += index - pvec.pages[i]->index - 1;
> + }
> }
I'm really not sure about your decision to have some interfaces expose
subpages and other expose huge pages. I think I'd be happier to see
all the existing interfaces made to continue exposing subpages, then
start adding in new interfaces and converting users one at a time
to use them. For example here, we'd add find_get_huge_pages_tag(),
then pagevec_lookup_huge_tag(), and switch this function from calling
pagevec_lookup_tag() to calling pagevec_lookup_huge_tag() ... then this
function is done; there's no messing around with calling compound_head
or PageTransHuge.
My dream is that eventually all callers will be able to cope with getting
a compound page back from the page cache and we can delete the versions
that return subpages, and rename the 'huge_' variations.
^ permalink raw reply
* Re: [PATCHv6 09/37] filemap: allocate huge page in pagecache_get_page(), if allowed
From: Matthew Wilcox @ 2017-02-09 21:59 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Andrew Morton,
Alexander Viro, Hugh Dickins, Andrea Arcangeli, Dave Hansen,
Vlastimil Babka, Ross Zwisler, linux-ext4, linux-fsdevel,
linux-kernel, linux-mm, linux-block
In-Reply-To: <20170126115819.58875-10-kirill.shutemov@linux.intel.com>
On Thu, Jan 26, 2017 at 02:57:51PM +0300, Kirill A. Shutemov wrote:
> Write path allocate pages using pagecache_get_page(). We should be able
> to allocate huge pages there, if it's allowed. As usually, fallback to
> small pages, if failed.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>
^ permalink raw reply
* Re: [PATCHv6 08/37] filemap: handle huge pages in do_generic_file_read()
From: Matthew Wilcox @ 2017-02-09 21:55 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Andrew Morton,
Alexander Viro, Hugh Dickins, Andrea Arcangeli, Dave Hansen,
Vlastimil Babka, Ross Zwisler, linux-ext4, linux-fsdevel,
linux-kernel, linux-mm, linux-block
In-Reply-To: <20170126115819.58875-9-kirill.shutemov@linux.intel.com>
On Thu, Jan 26, 2017 at 02:57:50PM +0300, Kirill A. Shutemov wrote:
> +++ b/mm/filemap.c
> @@ -1886,6 +1886,7 @@ static ssize_t do_generic_file_read(struct file *filp, loff_t *ppos,
> if (unlikely(page == NULL))
> goto no_cached_page;
> }
> + page = compound_head(page);
We got this page from find_get_page(), which gets it from
pagecache_get_page(), which gets it from find_get_entry() ... which
(unless I'm lost in your patch series) returns the head page. So this
line is redundant, right?
But then down in filemap_fault, we have:
VM_BUG_ON_PAGE(page->index != offset, page);
... again, maybe I'm lost somewhere in your patch series, but I don't see
anywhere you remove that line (or modify it). So are you not testing
with VM debugging enabled, or are you not doing a test which includes
mapping a file with huge pages, reading from it (to get the page in cache),
then faulting on an address that is not in the first 4kB of that 2MB?
^ permalink raw reply
* Re: v4.9, 4.4-final: 28 bioset threads on small notebook, 36 threads on cellphone
From: Kent Overstreet @ 2017-02-09 21:25 UTC (permalink / raw)
To: Mike Snitzer
Cc: Pavel Machek, kernel list, axboe, hch, neilb, martin.petersen,
dpark, ming.l, dm-devel, ming.lei, agk, jkosina, geoff, jim,
pjk1939, minchan, ngupta, oleg.drokin, andreas.dilger,
linux-block
In-Reply-To: <20170208163407.GA3646@redhat.com>
On Wed, Feb 08, 2017 at 11:34:07AM -0500, Mike Snitzer wrote:
> On Tue, Feb 07 2017 at 11:58pm -0500,
> Kent Overstreet <kent.overstreet@gmail.com> wrote:
>
> > On Tue, Feb 07, 2017 at 09:39:11PM +0100, Pavel Machek wrote:
> > > On Mon 2017-02-06 17:49:06, Kent Overstreet wrote:
> > > > On Mon, Feb 06, 2017 at 04:47:24PM -0900, Kent Overstreet wrote:
> > > > > On Mon, Feb 06, 2017 at 01:53:09PM +0100, Pavel Machek wrote:
> > > > > > Still there on v4.9, 36 threads on nokia n900 cellphone.
> > > > > >
> > > > > > So.. what needs to be done there?
> > > >
> > > > > But, I just got an idea for how to handle this that might be halfway sane, maybe
> > > > > I'll try and come up with a patch...
> > > >
> > > > Ok, here's such a patch, only lightly tested:
> > >
> > > I guess it would be nice for me to test it... but what it is against?
> > > I tried after v4.10-rc5 and linux-next, but got rejects in both cases.
> >
> > Sorry, I forgot I had a few other patches in my branch that touch
> > mempool/biosets code.
> >
> > Also, after thinking about it more and looking at the relevant code, I'm pretty
> > sure we don't need rescuer threads for block devices that just split bios - i.e.
> > most of them, so I changed my patch to do that.
> >
> > Tested it by ripping out the current->bio_list checks/workarounds from the
> > bcache code, appears to work:
>
> Feedback on this patch below, but first:
>
> There are deeper issues with the current->bio_list and rescue workqueues
> than thread counts.
>
> I cannot help but feel like you (and Jens) are repeatedly ignoring the
> issue that has been raised numerous times, most recently:
> https://www.redhat.com/archives/dm-devel/2017-February/msg00059.html
>
> FYI, this test (albeit ugly) can be used to check if the dm-snapshot
> deadlock is fixed:
> https://www.redhat.com/archives/dm-devel/2017-January/msg00064.html
>
> This situation is the unfortunate pathological worst case for what
> happens when changes are merged and nobody wants to own fixing the
> unforseen implications/regressions. Like everyone else in a position
> of Linux maintenance I've tried to stay away from owning the
> responsibility of a fix -- it isn't working. Ok, I'll stop bitching
> now.. I do bear responsibility for not digging in myself. We're all
> busy and this issue is "hard".
Mike, it's not my job to debug DM code for you or sift through your bug reports.
I don't read dm-devel, and I don't know why you think I that's my job.
If there's something you think the block layer should be doing differently, post
patches - or at the very least, explain what you'd like to be done, with words.
Don't get pissy because I'm not sifting through your bug reports.
Hell, I'm not getting paid to work on kernel code at all right now, and you
trying to rope me into fixing device mapper sure makes me want to work on the
block layer more.
DM developers have a long history of working siloed off from the rest of the
block layer, building up their own crazy infrastructure (remember the old bio
splitting code?) and going to extreme lengths to avoid having to work on or
improve the core block layer infrastructure. It's ridiculous.
You know what would be nice? What'd really make my day is if just once I got a
thank you or a bit of appreciation from DM developers for the bvec iterators/bio
splitting work I did that cleaned up a _lot_ of crazy hairy messes. Or getting
rid of merge_bvec_fn, or trying to come up with a better solution for deadlocks
due to running under generic_make_request() now.
^ permalink raw reply
* Re: [PATCHv6 07/37] filemap: allocate huge page in page_cache_read(), if allowed
From: Matthew Wilcox @ 2017-02-09 21:18 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Andrew Morton,
Alexander Viro, Hugh Dickins, Andrea Arcangeli, Dave Hansen,
Vlastimil Babka, Ross Zwisler, linux-ext4, linux-fsdevel,
linux-kernel, linux-mm, linux-block
In-Reply-To: <20170126115819.58875-8-kirill.shutemov@linux.intel.com>
On Thu, Jan 26, 2017 at 02:57:49PM +0300, Kirill A. Shutemov wrote:
> Later we can add logic to accumulate information from shadow entires to
> return to caller (average eviction time?).
I would say minimum rather than average. That will become the refault
time of the entire page, so minimum would probably have us making better
decisions?
> + /* Wipe shadow entires */
> + radix_tree_for_each_slot(slot, &mapping->page_tree, &iter,
> + page->index) {
> + if (iter.index >= page->index + hpage_nr_pages(page))
> + break;
>
> p = radix_tree_deref_slot_protected(slot, &mapping->tree_lock);
> - if (!radix_tree_exceptional_entry(p))
> + if (!p)
> + continue;
Just FYI, this can't happen. You're holding the tree lock so nobody
else gets to remove things from the tree. radix_tree_for_each_slot()
only gives you the full slots; it skips the empty ones for you. I'm
OK if you want to leave it in out of an abundance of caution.
> + __radix_tree_replace(&mapping->page_tree, iter.node, slot, NULL,
> + workingset_update_node, mapping);
I may add an update_node argument to radix_tree_join at some point,
so you can use it here. Or maybe we don't need to do that, and what
you have here works just fine.
> mapping->nrexceptional--;
... because adjusting the exceptional count is going to be a pain.
> + error = __radix_tree_insert(&mapping->page_tree,
> + page->index, compound_order(page), page);
> + /* This shouldn't happen */
> + if (WARN_ON_ONCE(error))
> + return error;
A lesser man would have just ignored the return value from
__radix_tree_insert. I salute you.
> @@ -2078,18 +2155,34 @@ static int page_cache_read(struct file *file, pgoff_t offset, gfp_t gfp_mask)
> {
> struct address_space *mapping = file->f_mapping;
> struct page *page;
> + pgoff_t hoffset;
> int ret;
>
> do {
> - page = __page_cache_alloc(gfp_mask|__GFP_COLD);
> + page = page_cache_alloc_huge(mapping, offset, gfp_mask);
> +no_huge:
> + if (!page)
> + page = __page_cache_alloc(gfp_mask|__GFP_COLD);
> if (!page)
> return -ENOMEM;
>
> - ret = add_to_page_cache_lru(page, mapping, offset, gfp_mask & GFP_KERNEL);
> - if (ret == 0)
> + if (PageTransHuge(page))
> + hoffset = round_down(offset, HPAGE_PMD_NR);
> + else
> + hoffset = offset;
> +
> + ret = add_to_page_cache_lru(page, mapping, hoffset,
> + gfp_mask & GFP_KERNEL);
> +
> + if (ret == -EEXIST && PageTransHuge(page)) {
> + put_page(page);
> + page = NULL;
> + goto no_huge;
> + } else if (ret == 0) {
> ret = mapping->a_ops->readpage(file, page);
> - else if (ret == -EEXIST)
> + } else if (ret == -EEXIST) {
> ret = 0; /* losing race to add is OK */
> + }
>
> put_page(page);
If the filesystem returns AOP_TRUNCATED_PAGE, you'll go round this loop
again trying the huge page again, even if the huge page didn't work
the first time. I would tend to think that if the huge page failed the
first time, we shouldn't try it again, so I propose this:
struct address_space *mapping = file->f_mapping;
struct page *page;
pgoff_t index;
int ret;
bool try_huge = true;
do {
if (try_huge) {
page = page_cache_alloc_huge(gfp_mask|__GFP_COLD);
if (page)
index = round_down(offset, HPAGE_PMD_NR);
else
try_huge = false;
}
if (!try_huge) {
page = __page_cache_alloc(gfp_mask|__GFP_COLD);
index = offset;
}
if (!page)
return -ENOMEM;
ret = add_to_page_cache_lru(page, mapping, index,
gfp_mask & GFP_KERNEL);
if (ret < 0) {
if (try_huge) {
try_huge = false;
ret = AOP_TRUNCATED_PAGE;
} else if (ret == -EEXIST)
ret = 0; /* losing race to add is OK */
} else {
ret = mapping->a_ops->readpage(file, page);
}
put_page(page);
} while (ret == AOP_TRUNCATED_PAGE);
But ... maybe it's OK to retry the huge page. I mean, not many
filesystems return AOP_TRUNCATED_PAGE, and they only do so rarely.
Anyway, I'm fine with the patch going in as-is. I just wanted to type out
my review notes.
Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>
^ permalink raw reply
* Re: [PATCHv6 06/37] thp: handle write-protection faults for file THP
From: Matthew Wilcox @ 2017-02-09 20:19 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Andrew Morton,
Alexander Viro, Hugh Dickins, Andrea Arcangeli, Dave Hansen,
Vlastimil Babka, Ross Zwisler, linux-ext4, linux-fsdevel,
linux-kernel, linux-mm, linux-block
In-Reply-To: <20170126115819.58875-7-kirill.shutemov@linux.intel.com>
On Thu, Jan 26, 2017 at 02:57:48PM +0300, Kirill A. Shutemov wrote:
> For filesystems that wants to be write-notified (has mkwrite), we will
> encount write-protection faults for huge PMDs in shared mappings.
>
> The easiest way to handle them is to clear the PMD and let it refault as
> wriable.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>
^ permalink raw reply
* Re: [PATCHv6 04/37] mm, rmap: account file thp pages
From: Matthew Wilcox @ 2017-02-09 20:17 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Andrew Morton,
Alexander Viro, Hugh Dickins, Andrea Arcangeli, Dave Hansen,
Vlastimil Babka, Ross Zwisler, linux-ext4, linux-fsdevel,
linux-kernel, linux-mm, linux-block
In-Reply-To: <20170126115819.58875-5-kirill.shutemov@linux.intel.com>
On Thu, Jan 26, 2017 at 02:57:46PM +0300, Kirill A. Shutemov wrote:
> Let's add FileHugePages and FilePmdMapped fields into meminfo and smaps.
> It indicates how many times we allocate and map file THP.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>
^ permalink raw reply
* Re: [PATCHv6 05/37] thp: try to free page's buffers before attempt split
From: Matthew Wilcox @ 2017-02-09 20:14 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, Andrew Morton,
Alexander Viro, Hugh Dickins, Andrea Arcangeli, Dave Hansen,
Vlastimil Babka, Ross Zwisler, linux-ext4, linux-fsdevel,
linux-kernel, linux-mm, linux-block
In-Reply-To: <20170126115819.58875-6-kirill.shutemov@linux.intel.com>
On Thu, Jan 26, 2017 at 02:57:47PM +0300, Kirill A. Shutemov wrote:
> @@ -2146,6 +2146,23 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
> goto out;
> }
>
> + /* Try to free buffers before attempt split */
> + if (!PageSwapBacked(head) && PagePrivate(page)) {
> + /*
> + * We cannot trigger writeback from here due possible
> + * recursion if triggered from vmscan, only wait.
> + *
> + * Caller can trigger writeback it on its own, if safe.
> + */
It took me a few reads to get this. May I suggest:
/*
* Cannot split a page with buffers. If the caller has
* already started writeback, we can wait for it to finish,
* but we cannot start writeback if we were called from vmscan
*/
> + if (!PageSwapBacked(head) && PagePrivate(page)) {
Also, it looks weird to test PageSwapBacked of *head* and PagePrivate
of *page*. I think it's correct, but it still looks weird.
Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>
^ permalink raw reply
* Re: Sed-opal fixups
From: Scott Bauer @ 2017-02-09 20:01 UTC (permalink / raw)
To: Jens Axboe
Cc: hch@infradead.org, arnd@arndb.de, linux-kernel@vger.kernel.org,
linux-nvme@lists.infradead.org, keith.busch@intel.com,
David Laight, linux-block@vger.kernel.org,
jonathan.derrick@intel.com
In-Reply-To: <45bcab45-3efa-e817-911a-ec3a0924dc6b@fb.com>
e0;136;0csOn Thu, Feb 09, 2017 at 11:24:58AM -0700, Jens Axboe wrote:
> On 02/09/2017 10:45 AM, Scott Bauer wrote:
> > On Thu, Feb 09, 2017 at 05:43:20PM +0000, David Laight wrote:
> >> From: Scott Bauer
> >>> Sent: 09 February 2017 17:20
> >>> It may be too late to change anyhting in the uapi header. When we
> >>> switched over to using IOC_SIZE I found a bug where I had switched
> >>> up a structure in one of the series from v4 to v5 but never changed
> >>> the structure in the IOW. The structure that was in there was to small
> >>> so when we kzalloc on it we don't request enough space. It worked before
> >>> because we were using the cmd strictly as a command #, not using the IOC
> >>> and friends.
> >>>
> >>> If it's too late to modify that IOW, I can work around it by reallocing
> >>> on the correct size for that command only. I verified the rest of the
> >>> commands and the structures are the same.
> >>>
> >>> Let me know what you think, please.
> >>
> >> Maybe define IOC_OPAL_ACTIVATE_LSP_OLD to the incorrect value and
> >> IOC_OPAL_ACTIVATE_LSP to the correct one.
> >> But that relies on any users specifying the correct structure.
> >> I wouldn't guarantee that.
> >
> > I think I'm the only userspace user right now, this went in on monday,
> > so I can can change my tooling easily. I just wasnt sure if there was a
> > set time where the user ABI cannot be changed.
>
> We can still change it, and we definitely should if it improves the
> interface. It's not a ABI until it's in a released, final kernel.
>
Thanks for the clarification, Jens.
In that case I'd like to keep the ABI fixup in patch one and the dynamic allocation
in patch 2.
I'd like to wait for Christoph's blessing as well before you take it.
Thanks
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply
* Re: [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
From: Rafael Antognolli @ 2017-02-09 19:51 UTC (permalink / raw)
To: Scott Bauer
Cc: hch, arnd, axboe, linux-kernel, linux-nvme, keith.busch,
David.Laight, linux-block, jonathan.derrick
In-Reply-To: <1486660801-5105-3-git-send-email-scott.bauer@intel.com>
On Thu, Feb 09, 2017 at 10:20:01AM -0700, Scott Bauer wrote:
> When CONFIG_KASAN is enabled, compilation fails:
>
> block/sed-opal.c: In function 'sed_ioctl':
> block/sed-opal.c:2447:1: error: the frame size of 2256 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
>
> Moved all the ioctl structures off the stack and dynamically activate
> using _IOC_SIZE()
I just went through the other threads about this issue. This approach
looks good to me.
Rafael
> Fixes: 455a7b238cd6 ("block: Add Sed-opal library")
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Scott Bauer <scott.bauer@intel.com>
> ---
> block/sed-opal.c | 134 +++++++++++++++++++++----------------------------------
> 1 file changed, 50 insertions(+), 84 deletions(-)
>
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index bf1406e..4985d95 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -2346,7 +2346,10 @@ EXPORT_SYMBOL(opal_unlock_from_suspend);
>
> int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
> {
> + void *ioctl_ptr;
> + int ret = -ENOTTY;
> void __user *arg = (void __user *)ptr;
> + unsigned int cmd_size = _IOC_SIZE(cmd);
>
> if (!capable(CAP_SYS_ADMIN))
> return -EACCES;
> @@ -2355,94 +2358,57 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
> return -ENOTSUPP;
> }
>
> - switch (cmd) {
> - case IOC_OPAL_SAVE: {
> - struct opal_lock_unlock lk_unlk;
> -
> - if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
> - return -EFAULT;
> - return opal_save(dev, &lk_unlk);
> - }
> - case IOC_OPAL_LOCK_UNLOCK: {
> - struct opal_lock_unlock lk_unlk;
> -
> - if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
> - return -EFAULT;
> - return opal_lock_unlock(dev, &lk_unlk);
> - }
> - case IOC_OPAL_TAKE_OWNERSHIP: {
> - struct opal_key opal_key;
> -
> - if (copy_from_user(&opal_key, arg, sizeof(opal_key)))
> - return -EFAULT;
> - return opal_take_ownership(dev, &opal_key);
> - }
> - case IOC_OPAL_ACTIVATE_LSP: {
> - struct opal_lr_act opal_lr_act;
> -
> - if (copy_from_user(&opal_lr_act, arg, sizeof(opal_lr_act)))
> - return -EFAULT;
> - return opal_activate_lsp(dev, &opal_lr_act);
> - }
> - case IOC_OPAL_SET_PW: {
> - struct opal_new_pw opal_pw;
> -
> - if (copy_from_user(&opal_pw, arg, sizeof(opal_pw)))
> - return -EFAULT;
> - return opal_set_new_pw(dev, &opal_pw);
> - }
> - case IOC_OPAL_ACTIVATE_USR: {
> - struct opal_session_info session;
> -
> - if (copy_from_user(&session, arg, sizeof(session)))
> - return -EFAULT;
> - return opal_activate_user(dev, &session);
> - }
> - case IOC_OPAL_REVERT_TPR: {
> - struct opal_key opal_key;
> -
> - if (copy_from_user(&opal_key, arg, sizeof(opal_key)))
> - return -EFAULT;
> - return opal_reverttper(dev, &opal_key);
> - }
> - case IOC_OPAL_LR_SETUP: {
> - struct opal_user_lr_setup lrs;
> -
> - if (copy_from_user(&lrs, arg, sizeof(lrs)))
> - return -EFAULT;
> - return opal_setup_locking_range(dev, &lrs);
> - }
> - case IOC_OPAL_ADD_USR_TO_LR: {
> - struct opal_lock_unlock lk_unlk;
> -
> - if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
> - return -EFAULT;
> - return opal_add_user_to_lr(dev, &lk_unlk);
> - }
> - case IOC_OPAL_ENABLE_DISABLE_MBR: {
> - struct opal_mbr_data mbr;
> -
> - if (copy_from_user(&mbr, arg, sizeof(mbr)))
> - return -EFAULT;
> - return opal_enable_disable_shadow_mbr(dev, &mbr);
> - }
> - case IOC_OPAL_ERASE_LR: {
> - struct opal_session_info session;
> -
> - if (copy_from_user(&session, arg, sizeof(session)))
> - return -EFAULT;
> - return opal_erase_locking_range(dev, &session);
> + ioctl_ptr = kzalloc(cmd_size, GFP_KERNEL);
> + if (!ioctl_ptr)
> + return -ENOMEM;
> + if (copy_from_user(ioctl_ptr, arg, cmd_size)) {
> + ret = -EFAULT;
> + goto out;
> }
> - case IOC_OPAL_SECURE_ERASE_LR: {
> - struct opal_session_info session;
>
> - if (copy_from_user(&session, arg, sizeof(session)))
> - return -EFAULT;
> - return opal_secure_erase_locking_range(dev, &session);
> - }
> + switch (cmd) {
> + case IOC_OPAL_SAVE:
> + ret = opal_save(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_LOCK_UNLOCK:
> + ret = opal_lock_unlock(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_TAKE_OWNERSHIP:
> + ret = opal_take_ownership(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_ACTIVATE_LSP:
> + ret = opal_activate_lsp(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_SET_PW:
> + ret = opal_set_new_pw(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_ACTIVATE_USR:
> + ret = opal_activate_user(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_REVERT_TPR:
> + ret = opal_reverttper(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_LR_SETUP:
> + ret = opal_setup_locking_range(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_ADD_USR_TO_LR:
> + ret = opal_add_user_to_lr(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_ENABLE_DISABLE_MBR:
> + ret = opal_enable_disable_shadow_mbr(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_ERASE_LR:
> + ret = opal_erase_locking_range(dev, ioctl_ptr);
> + break;
> + case IOC_OPAL_SECURE_ERASE_LR:
> + ret = opal_secure_erase_locking_range(dev, ioctl_ptr);
> + break;
> default:
> pr_warn("No such Opal Ioctl %u\n", cmd);
> }
> - return -ENOTTY;
> +
> +out:
> + kfree(ioctl_ptr);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(sed_ioctl);
> --
> 2.7.4
>
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply
* Re: Sed-opal fixups
From: Jens Axboe @ 2017-02-09 18:24 UTC (permalink / raw)
To: Scott Bauer, David Laight
Cc: keith.busch@intel.com, arnd@arndb.de, hch@infradead.org,
linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
linux-block@vger.kernel.org, jonathan.derrick@intel.com
In-Reply-To: <20170209174526.GA5200@sbauer-Z170X-UD5>
On 02/09/2017 10:45 AM, Scott Bauer wrote:
> On Thu, Feb 09, 2017 at 05:43:20PM +0000, David Laight wrote:
>> From: Scott Bauer
>>> Sent: 09 February 2017 17:20
>>> It may be too late to change anyhting in the uapi header. When we
>>> switched over to using IOC_SIZE I found a bug where I had switched
>>> up a structure in one of the series from v4 to v5 but never changed
>>> the structure in the IOW. The structure that was in there was to small
>>> so when we kzalloc on it we don't request enough space. It worked before
>>> because we were using the cmd strictly as a command #, not using the IOC
>>> and friends.
>>>
>>> If it's too late to modify that IOW, I can work around it by reallocing
>>> on the correct size for that command only. I verified the rest of the
>>> commands and the structures are the same.
>>>
>>> Let me know what you think, please.
>>
>> Maybe define IOC_OPAL_ACTIVATE_LSP_OLD to the incorrect value and
>> IOC_OPAL_ACTIVATE_LSP to the correct one.
>> But that relies on any users specifying the correct structure.
>> I wouldn't guarantee that.
>
> I think I'm the only userspace user right now, this went in on monday,
> so I can can change my tooling easily. I just wasnt sure if there was a
> set time where the user ABI cannot be changed.
We can still change it, and we definitely should if it improves the
interface. It's not a ABI until it's in a released, final kernel.
--
Jens Axboe
^ permalink raw reply
* Re: Sed-opal fixups
From: Scott Bauer @ 2017-02-09 17:45 UTC (permalink / raw)
To: David Laight
Cc: keith.busch@intel.com, arnd@arndb.de, hch@infradead.org,
linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
axboe@fb.com, linux-block@vger.kernel.org,
jonathan.derrick@intel.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0281B85@AcuExch.aculab.com>
On Thu, Feb 09, 2017 at 05:43:20PM +0000, David Laight wrote:
> From: Scott Bauer
> > Sent: 09 February 2017 17:20
> > It may be too late to change anyhting in the uapi header. When we
> > switched over to using IOC_SIZE I found a bug where I had switched
> > up a structure in one of the series from v4 to v5 but never changed
> > the structure in the IOW. The structure that was in there was to small
> > so when we kzalloc on it we don't request enough space. It worked before
> > because we were using the cmd strictly as a command #, not using the IOC
> > and friends.
> >
> > If it's too late to modify that IOW, I can work around it by reallocing
> > on the correct size for that command only. I verified the rest of the
> > commands and the structures are the same.
> >
> > Let me know what you think, please.
>
> Maybe define IOC_OPAL_ACTIVATE_LSP_OLD to the incorrect value and
> IOC_OPAL_ACTIVATE_LSP to the correct one.
> But that relies on any users specifying the correct structure.
> I wouldn't guarantee that.
I think I'm the only userspace user right now, this went in on monday,
so I can can change my tooling easily. I just wasnt sure if there was a
set time where the user ABI cannot be changed.
>
> At the top of the driver's ioctl path add:
> if (cmd == IOC_OPAL_ACTIVATE_LSP_OLD) cmd = IOC_OPAL_ACTIVATE_LSP;
>
I think it would have to be the other way around the correct sized one would
be IOC_OPAL_ACTIAVE_LSP_NEW so the check would be:
if (cmd == IOC_OPAL_ACTIVATE_LSP) cmd = IOC_OPAL_ACTIVATE_LSP_NEW. If we're
allowed to change it (the bad sized one) from LSP to LSP_OLD then we should
just change the structure. If we have to leave it we need to introduce a _NEW
with the correct size.
> For some code I added a userspace wrapper on ioctl() to check the
> size of the supplied arg matched that required by the 'cmd'.
> I've also done the same in the kernel.
> (all as compile time checks).
>
> David
>
>
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply
* RE: Sed-opal fixups
From: David Laight @ 2017-02-09 17:43 UTC (permalink / raw)
To: 'Scott Bauer', linux-nvme@lists.infradead.org
Cc: keith.busch@intel.com, arnd@arndb.de, hch@infradead.org,
linux-kernel@vger.kernel.org, axboe@fb.com,
linux-block@vger.kernel.org, jonathan.derrick@intel.com
In-Reply-To: <1486660801-5105-1-git-send-email-scott.bauer@intel.com>
From: Scott Bauer
> Sent: 09 February 2017 17:20
> It may be too late to change anyhting in the uapi header. When we
> switched over to using IOC_SIZE I found a bug where I had switched
> up a structure in one of the series from v4 to v5 but never changed
> the structure in the IOW. The structure that was in there was to small
> so when we kzalloc on it we don't request enough space. It worked before
> because we were using the cmd strictly as a command #, not using the IOC
> and friends.
>
> If it's too late to modify that IOW, I can work around it by reallocing
> on the correct size for that command only. I verified the rest of the
> commands and the structures are the same.
>
> Let me know what you think, please.
Maybe define IOC_OPAL_ACTIVATE_LSP_OLD to the incorrect value and
IOC_OPAL_ACTIVATE_LSP to the correct one.
But that relies on any users specifying the correct structure.
I wouldn't guarantee that.
At the top of the driver's ioctl path add:
if (cmd == IOC_OPAL_ACTIVATE_LSP_OLD) cmd = IOC_OPAL_ACTIVATE_LSP;
For some code I added a userspace wrapper on ioctl() to check the
size of the supplied arg matched that required by the 'cmd'.
I've also done the same in the kernel.
(all as compile time checks).
David
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply
* [PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN
From: Scott Bauer @ 2017-02-09 17:20 UTC (permalink / raw)
To: linux-nvme
Cc: keith.busch, arnd, hch, linux-kernel, axboe, David.Laight,
linux-block, Scott Bauer, jonathan.derrick
In-Reply-To: <1486660801-5105-1-git-send-email-scott.bauer@intel.com>
When CONFIG_KASAN is enabled, compilation fails:
block/sed-opal.c: In function 'sed_ioctl':
block/sed-opal.c:2447:1: error: the frame size of 2256 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
Moved all the ioctl structures off the stack and dynamically activate
using _IOC_SIZE()
Fixes: 455a7b238cd6 ("block: Add Sed-opal library")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
---
block/sed-opal.c | 134 +++++++++++++++++++++----------------------------------
1 file changed, 50 insertions(+), 84 deletions(-)
diff --git a/block/sed-opal.c b/block/sed-opal.c
index bf1406e..4985d95 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -2346,7 +2346,10 @@ EXPORT_SYMBOL(opal_unlock_from_suspend);
int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
{
+ void *ioctl_ptr;
+ int ret = -ENOTTY;
void __user *arg = (void __user *)ptr;
+ unsigned int cmd_size = _IOC_SIZE(cmd);
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
@@ -2355,94 +2358,57 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
return -ENOTSUPP;
}
- switch (cmd) {
- case IOC_OPAL_SAVE: {
- struct opal_lock_unlock lk_unlk;
-
- if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
- return -EFAULT;
- return opal_save(dev, &lk_unlk);
- }
- case IOC_OPAL_LOCK_UNLOCK: {
- struct opal_lock_unlock lk_unlk;
-
- if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
- return -EFAULT;
- return opal_lock_unlock(dev, &lk_unlk);
- }
- case IOC_OPAL_TAKE_OWNERSHIP: {
- struct opal_key opal_key;
-
- if (copy_from_user(&opal_key, arg, sizeof(opal_key)))
- return -EFAULT;
- return opal_take_ownership(dev, &opal_key);
- }
- case IOC_OPAL_ACTIVATE_LSP: {
- struct opal_lr_act opal_lr_act;
-
- if (copy_from_user(&opal_lr_act, arg, sizeof(opal_lr_act)))
- return -EFAULT;
- return opal_activate_lsp(dev, &opal_lr_act);
- }
- case IOC_OPAL_SET_PW: {
- struct opal_new_pw opal_pw;
-
- if (copy_from_user(&opal_pw, arg, sizeof(opal_pw)))
- return -EFAULT;
- return opal_set_new_pw(dev, &opal_pw);
- }
- case IOC_OPAL_ACTIVATE_USR: {
- struct opal_session_info session;
-
- if (copy_from_user(&session, arg, sizeof(session)))
- return -EFAULT;
- return opal_activate_user(dev, &session);
- }
- case IOC_OPAL_REVERT_TPR: {
- struct opal_key opal_key;
-
- if (copy_from_user(&opal_key, arg, sizeof(opal_key)))
- return -EFAULT;
- return opal_reverttper(dev, &opal_key);
- }
- case IOC_OPAL_LR_SETUP: {
- struct opal_user_lr_setup lrs;
-
- if (copy_from_user(&lrs, arg, sizeof(lrs)))
- return -EFAULT;
- return opal_setup_locking_range(dev, &lrs);
- }
- case IOC_OPAL_ADD_USR_TO_LR: {
- struct opal_lock_unlock lk_unlk;
-
- if (copy_from_user(&lk_unlk, arg, sizeof(lk_unlk)))
- return -EFAULT;
- return opal_add_user_to_lr(dev, &lk_unlk);
- }
- case IOC_OPAL_ENABLE_DISABLE_MBR: {
- struct opal_mbr_data mbr;
-
- if (copy_from_user(&mbr, arg, sizeof(mbr)))
- return -EFAULT;
- return opal_enable_disable_shadow_mbr(dev, &mbr);
- }
- case IOC_OPAL_ERASE_LR: {
- struct opal_session_info session;
-
- if (copy_from_user(&session, arg, sizeof(session)))
- return -EFAULT;
- return opal_erase_locking_range(dev, &session);
+ ioctl_ptr = kzalloc(cmd_size, GFP_KERNEL);
+ if (!ioctl_ptr)
+ return -ENOMEM;
+ if (copy_from_user(ioctl_ptr, arg, cmd_size)) {
+ ret = -EFAULT;
+ goto out;
}
- case IOC_OPAL_SECURE_ERASE_LR: {
- struct opal_session_info session;
- if (copy_from_user(&session, arg, sizeof(session)))
- return -EFAULT;
- return opal_secure_erase_locking_range(dev, &session);
- }
+ switch (cmd) {
+ case IOC_OPAL_SAVE:
+ ret = opal_save(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_LOCK_UNLOCK:
+ ret = opal_lock_unlock(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_TAKE_OWNERSHIP:
+ ret = opal_take_ownership(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_ACTIVATE_LSP:
+ ret = opal_activate_lsp(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_SET_PW:
+ ret = opal_set_new_pw(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_ACTIVATE_USR:
+ ret = opal_activate_user(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_REVERT_TPR:
+ ret = opal_reverttper(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_LR_SETUP:
+ ret = opal_setup_locking_range(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_ADD_USR_TO_LR:
+ ret = opal_add_user_to_lr(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_ENABLE_DISABLE_MBR:
+ ret = opal_enable_disable_shadow_mbr(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_ERASE_LR:
+ ret = opal_erase_locking_range(dev, ioctl_ptr);
+ break;
+ case IOC_OPAL_SECURE_ERASE_LR:
+ ret = opal_secure_erase_locking_range(dev, ioctl_ptr);
+ break;
default:
pr_warn("No such Opal Ioctl %u\n", cmd);
}
- return -ENOTTY;
+
+out:
+ kfree(ioctl_ptr);
+ return ret;
}
EXPORT_SYMBOL_GPL(sed_ioctl);
--
2.7.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related
* [PATCH V3 1/2] uapi: sed-opal fix IOW for activate lsp to use correct struct
From: Scott Bauer @ 2017-02-09 17:20 UTC (permalink / raw)
To: linux-nvme
Cc: keith.busch, arnd, hch, linux-kernel, axboe, David.Laight,
linux-block, Scott Bauer, jonathan.derrick
In-Reply-To: <1486660801-5105-1-git-send-email-scott.bauer@intel.com>
the IOW for the IOC_OPAL_ACTIVATE_LSP took the wrong strcure which
would give us the wrong size when using _IOC_SIZE, switch it to the
right structure.
Fixes: 058f8a2 ("Include: Uapi: Add user ABI for Sed/Opal")
Signed-off-by: Scott Bauer <scott.bauer@intel.com>
---
include/uapi/linux/sed-opal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index fc06e3a..c72e073 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -106,7 +106,7 @@ struct opal_mbr_data {
#define IOC_OPAL_SAVE _IOW('p', 220, struct opal_lock_unlock)
#define IOC_OPAL_LOCK_UNLOCK _IOW('p', 221, struct opal_lock_unlock)
#define IOC_OPAL_TAKE_OWNERSHIP _IOW('p', 222, struct opal_key)
-#define IOC_OPAL_ACTIVATE_LSP _IOW('p', 223, struct opal_key)
+#define IOC_OPAL_ACTIVATE_LSP _IOW('p', 223, struct opal_lr_act)
#define IOC_OPAL_SET_PW _IOW('p', 224, struct opal_new_pw)
#define IOC_OPAL_ACTIVATE_USR _IOW('p', 225, struct opal_session_info)
#define IOC_OPAL_REVERT_TPR _IOW('p', 226, struct opal_key)
--
2.7.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related
* Sed-opal fixups
From: Scott Bauer @ 2017-02-09 17:19 UTC (permalink / raw)
To: linux-nvme
Cc: keith.busch, arnd, hch, linux-kernel, axboe, David.Laight,
linux-block, jonathan.derrick
It may be too late to change anyhting in the uapi header. When we
switched over to using IOC_SIZE I found a bug where I had switched
up a structure in one of the series from v4 to v5 but never changed
the structure in the IOW. The structure that was in there was to small
so when we kzalloc on it we don't request enough space. It worked before
because we were using the cmd strictly as a command #, not using the IOC
and friends.
If it's too late to modify that IOW, I can work around it by reallocing
on the correct size for that command only. I verified the rest of the
commands and the structures are the same.
Let me know what you think, please.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply
* Re: [PATCHv6 01/37] mm, shmem: swich huge tmpfs to multi-order radix-tree entries
From: Kirill A. Shutemov @ 2017-02-09 16:58 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kirill A. Shutemov, Theodore Ts'o, Andreas Dilger, Jan Kara,
Andrew Morton, Alexander Viro, Hugh Dickins, Andrea Arcangeli,
Dave Hansen, Vlastimil Babka, Ross Zwisler, linux-ext4,
linux-fsdevel, linux-kernel, linux-mm, linux-block
In-Reply-To: <20170209035727.GQ2267@bombadil.infradead.org>
On Wed, Feb 08, 2017 at 07:57:27PM -0800, Matthew Wilcox wrote:
> On Thu, Jan 26, 2017 at 02:57:43PM +0300, Kirill A. Shutemov wrote:
> > +++ b/include/linux/pagemap.h
> > @@ -332,6 +332,15 @@ static inline struct page *grab_cache_page_nowait(struct address_space *mapping,
> > mapping_gfp_mask(mapping));
> > }
> >
> > +static inline struct page *find_subpage(struct page *page, pgoff_t offset)
> > +{
> > + VM_BUG_ON_PAGE(PageTail(page), page);
> > + VM_BUG_ON_PAGE(page->index > offset, page);
> > + VM_BUG_ON_PAGE(page->index + (1 << compound_order(page)) < offset,
> > + page);
> > + return page - page->index + offset;
> > +}
>
> What would you think to:
>
> static inline void check_page_index(struct page *page, pgoff_t offset)
> {
> VM_BUG_ON_PAGE(PageTail(page), page);
> VM_BUG_ON_PAGE(page->index > offset, page);
> VM_BUG_ON_PAGE(page->index + (1 << compound_order(page)) <= offset,
> page);
> }
>
> (I think I fixed an off-by-one error up there ... if
> index + (1 << order) == offset, this is also a bug, right?
> because offset would then refer to the next page, not this page)
Right, thanks.
>
> static inline struct page *find_subpage(struct page *page, pgoff_t offset)
> {
> check_page_index(page, offset);
> return page + (offset - page->index);
> }
>
> ... then you can use check_page_index down ...
Okay, makes sense.
>
> > @@ -1250,7 +1233,6 @@ struct page *find_lock_entry(struct address_space *mapping, pgoff_t offset)
> > put_page(page);
> > goto repeat;
> > }
> > - VM_BUG_ON_PAGE(page_to_pgoff(page) != offset, page);
>
> ... here?
Ok.
> > @@ -1472,25 +1451,35 @@ unsigned find_get_pages(struct address_space *mapping, pgoff_t start,
> > goto repeat;
> > }
> >
> > + /* For multi-order entries, find relevant subpage */
> > + if (PageTransHuge(page)) {
> > + VM_BUG_ON(index - page->index < 0);
> > + VM_BUG_ON(index - page->index >= HPAGE_PMD_NR);
> > + page += index - page->index;
> > + }
>
> Use find_subpage() here?
Ok.
> > pages[ret] = page;
> > if (++ret == nr_pages)
> > break;
> > + if (!PageTransCompound(page))
> > + continue;
> > + for (refs = 0; ret < nr_pages &&
> > + (index + 1) % HPAGE_PMD_NR;
> > + ret++, refs++, index++)
> > + pages[ret] = ++page;
> > + if (refs)
> > + page_ref_add(compound_head(page), refs);
> > + if (ret == nr_pages)
> > + break;
>
> Can we avoid referencing huge pages specifically in the page cache? I'd
> like us to get to the point where we can put arbitrary compound pages into
> the page cache. For example, I think this can be written as:
>
> if (!PageCompound(page))
> continue;
> for (refs = 0; ret < nr_pages; refs++, index++) {
> if (index > page->index + (1 << compound_order(page)))
> break;
> pages[ret++] = ++page;
> }
> if (refs)
> page_ref_add(compound_head(page), refs);
> if (ret == nr_pages)
> break;
That's slightly more costly, but I guess that's fine.
> > @@ -1541,19 +1533,12 @@ unsigned find_get_pages_contig(struct address_space *mapping, pgoff_t index,
> >
> > + /* For multi-order entries, find relevant subpage */
> > + if (PageTransHuge(page)) {
> > + VM_BUG_ON(index - page->index < 0);
> > + VM_BUG_ON(index - page->index >= HPAGE_PMD_NR);
> > + page += index - page->index;
> > + }
> > +
> > pages[ret] = page;
> > if (++ret == nr_pages)
> > break;
> > + if (!PageTransCompound(page))
> > + continue;
> > + for (refs = 0; ret < nr_pages &&
> > + (index + 1) % HPAGE_PMD_NR;
> > + ret++, refs++, index++)
> > + pages[ret] = ++page;
> > + if (refs)
> > + page_ref_add(compound_head(page), refs);
> > + if (ret == nr_pages)
> > + break;
> > }
> > rcu_read_unlock();
> > return ret;
>
> Ugh, the same code again. Hmm ... we only need to modify 'ret' as a result
> of this ... so could we split it out like this?
>
> static unsigned populate_pages(struct page **pages, unsigned i, unsigned max,
> struct page *page)
> {
> unsigned refs = 0;
> for (;;) {
> pages[i++] = page;
> if (i == max)
> break;
> if (PageHead(page + 1))
> break;
Hm? PageHead()? No. The next page can head or small.
I *guess* we can get away with !PageTail(page + 1)...
> page++;
> refs++;
> }
> if (refs)
> page_ref_add(compound_head(page), refs);
> return i;
> }
>
> > +unsigned find_get_pages_tag(struct address_space *mapping, pgoff_t *indexp,
> > int tag, unsigned int nr_pages, struct page **pages)
>
> > break;
> > + if (!PageTransCompound(page))
> > + continue;
> > + for (refs = 0; ret < nr_pages &&
> > + (index + 1) % HPAGE_PMD_NR;
> > + ret++, refs++, index++)
> > + pages[ret] = ++page;
> > + if (refs)
> > + page_ref_add(compound_head(page), refs);
> > + if (ret == nr_pages)
> > + break;
> > }
>
> ... and again!
>
> > @@ -2326,25 +2337,26 @@ void filemap_map_pages(struct vm_fault *vmf,
> > + /* For multi-order entries, find relevant subpage */
> > + if (PageTransHuge(page)) {
> > + VM_BUG_ON(index - page->index < 0);
> > + VM_BUG_ON(index - page->index >= HPAGE_PMD_NR);
> > + page += index - page->index;
> > + }
> > +
> > if (!PageUptodate(page) ||
> > PageReadahead(page) ||
>
> Readahead is PF_NO_COMPOUND ... so I don't see why this works?
We wouldn't see pages with readahead flag set/cleared until huge pages
support in ext4 is enabled at very end of the patchset.
>
> > @@ -2378,8 +2390,14 @@ void filemap_map_pages(struct vm_fault *vmf,
> > /* Huge page is mapped? No need to proceed. */
> > if (pmd_trans_huge(*vmf->pmd))
> > break;
> > - if (iter.index == end_pgoff)
> > + if (index == end_pgoff)
> > break;
> > + if (page && PageTransCompound(page) &&
> > + (index & (HPAGE_PMD_NR - 1)) !=
> > + HPAGE_PMD_NR - 1) {
> > + index++;
> > + goto repeat;
>
> Do we really have to go all the way back to the beginning of the loop? It'd
> be nice to be able to insert all of the relevant PTEs in a shorter loop here.
> We'd need to bump the reference count N more times, and I think we do need
> to check HWPoison for each subpage.
I'll look into it.
Thanks for review.
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 1/1] dm-zoned: Zoned block device target
From: Hannes Reinecke @ 2017-02-09 16:07 UTC (permalink / raw)
To: Damien Le Moal, dm-devel, Alasdair Kergon, Mike Snitzer
Cc: Christoph Hellwig, linux-block
In-Reply-To: <20170209041849.30964-1-damien.lemoal@wdc.com>
On 02/09/2017 05:18 AM, Damien Le Moal wrote:
> The dm-zoned device mapper target provides transparent write access
> to zoned block devices (ZBC and ZAC compliant block devices).
> dm-zoned hides to the device user (a file system or an application
> doing raw block device accesses) any constraint imposed on write
> requests by the device. Write requests are processed using a
> combination of on-disk buffering using the device conventional zones,
> allowing any random write access to be safely executed, or direct in-place
> processing for requests aligned on a zone sequential write pointer position.
> A background reclaim process ensures that some conventional zones are always
> available for executing unaligned write requests. The reclaim process
> overhead is minimized by managing buffer zones in a least-recently-written
> order and first targeting the oldest buffer zones. Doing so, blocks under
> regular write access (such as metadata blocks of an FS) remain stored in
> conventional zones, resulting in no apparent write overhead.
>
[ .. ]
> +/*
> + * CRC32
> + */
> +static u32 dmz_sb_crc32(u32 crc, const void *buf, size_t length)
> +{
> + unsigned char *p = (unsigned char *)buf;
> + int i;
> +
> +#define CRCPOLY_LE 0xedb88320
> +
> + while (length--) {
> + crc ^= *p++;
> + for (i = 0; i < 8; i++)
> + crc = (crc >> 1) ^ ((crc & 1) ? CRCPOLY_LE : 0);
> + }
> +
> + return crc;
> +}
> +
Why do you insist on your own CRC32 implementations?
Don't we have enough already?
[ .. ]
> +/*
> + * Write blocks.
> + */
> +static int dmz_reclaim_write(struct dm_zoned_target *dzt,
> + struct dm_zone *zone,
> + struct dm_zoned_ioreg **ioregs,
> + unsigned int nr_ioregs)
> +{
> + struct dm_zoned_ioreg *ioreg;
> + sector_t chunk_block;
> + int i, ret = 0;
> +
> + for (i = 0; i < nr_ioregs; i++) {
> +
> + ioreg = ioregs[i];
> +
> + /* Wait for the read I/O to complete */
> + wait_for_completion_io(&ioreg->wait);
> +
> + if (ret || ioreg->err) {
> + if (ret == 0)
> + ret = ioreg->err;
> + dmz_reclaim_free_ioreg(ioreg);
> + ioregs[i] = NULL;
> + continue;
> + }
> +
> + chunk_block = ioreg->chunk_block;
> +
> + dmz_dev_debug(dzt,
> + "Reclaim: Write %s zone %u, block %llu+%u\n",
> + dmz_is_rnd(zone) ? "RND" : "SEQ",
> + dmz_id(dzt, zone),
> + (unsigned long long)chunk_block,
> + ioreg->nr_blocks);
> +
> + /*
> + * If we are writing in a sequential zones,
> + * we must make sure that writes are sequential. So
> + * fill up any eventual hole between writes.
> + */
> + if (dmz_is_seq(zone)) {
> + ret = dmz_reclaim_align_wp(dzt, zone, chunk_block);
> + if (ret)
> + break;
> + }
> +
> + /* Do write */
> + dmz_reclaim_submit_ioreg(dzt, zone, ioreg, REQ_OP_WRITE);
> + wait_for_completion_io(&ioreg->wait);
> +
> + ret = ioreg->err;
> + if (ret) {
> + dmz_dev_err(dzt, "Reclaim: Write failed\n");
> + } else {
> + if (dmz_is_seq(zone))
> + zone->wp_block += ioreg->nr_blocks;
> + ret = dmz_validate_blocks(dzt, zone, chunk_block,
> + ioreg->nr_blocks);
> + }
> +
> + ioregs[i] = NULL;
> + dmz_reclaim_free_ioreg(ioreg);
> +
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * Move valid blocks of src_zone into dst_zone.
> + */
> +static int dmz_reclaim_copy_zone(struct dm_zoned_target *dzt,
> + struct dm_zone *src_zone,
> + struct dm_zone *dst_zone)
> +{
> + struct dm_zoned_ioreg *ioregs[DMZ_RECLAIM_MAX_IOREGS];
> + struct dm_zoned_ioreg *ioreg;
> + sector_t chunk_block = 0;
> + sector_t first_block, end_block;
> + int nr_ioregs = 0, i, ret;
> +
> + if (dmz_is_seq(src_zone))
> + end_block = src_zone->wp_block;
> + else
> + end_block = dzt->zone_nr_blocks;
> +
> + while (chunk_block < end_block) {
> +
> + /* Read valid regions from source zone */
> + nr_ioregs = 0;
> + first_block = chunk_block;
> + while (nr_ioregs < DMZ_RECLAIM_MAX_IOREGS &&
> + chunk_block < end_block) {
> +
> + ioreg = dmz_reclaim_read(dzt, src_zone, chunk_block);
> + if (IS_ERR(ioreg)) {
> + ret = PTR_ERR(ioreg);
> + goto err;
> + }
> + if (!ioreg)
> + break;
> +
> + chunk_block = ioreg->chunk_block + ioreg->nr_blocks;
> + ioregs[nr_ioregs] = ioreg;
> + nr_ioregs++;
> +
> + }
> +
> + /* Are we done ? */
> + if (!nr_ioregs)
> + break;
> +
> + /* Write in destination zone */
> + ret = dmz_reclaim_write(dzt, dst_zone, ioregs, nr_ioregs);
> + if (ret != 0) {
> + dmz_invalidate_blocks(dzt, dst_zone,
> + first_block,
> + end_block - first_block);
> + goto err;
> + }
> +
> + }
> +
> + return 0;
> +
> +err:
> + for (i = 0; i < nr_ioregs; i++) {
> + ioreg = ioregs[i];
> + if (ioreg) {
> + wait_for_completion_io(&ioreg->wait);
> + dmz_reclaim_free_ioreg(ioreg);
> + }
> + }
> +
> + return ret;
> +}
> +
Have you looked at using dm-kcopyd for doing the reclaim?
What's inhibiting you from using it?
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: F. Imend�rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N�rnberg)
^ permalink raw reply
* Re: automatic IRQ affinity for virtio V3
From: Michael S. Tsirkin @ 2017-02-09 16:01 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Christoph Hellwig, jasowang, axboe, pbonzini, virtualization,
linux-block, linux-kernel
In-Reply-To: <20170209135031.GA5771@infradead.org>
On Thu, Feb 09, 2017 at 05:50:31AM -0800, Christoph Hellwig wrote:
> On Sun, Feb 05, 2017 at 06:15:17PM +0100, Christoph Hellwig wrote:
> > Hi Michael, hi Jason,
> >
> > This patches applies a few cleanups to the virtio PCI interrupt handling
> > code, and then converts the virtio PCI code to use the automatic MSI-X
> > vectors spreading, as well as using the information in virtio-blk
> > and virtio-scsi to automatically align the blk-mq queues to the MSI-X
> > vectors.
>
> Any chance to get this in for 4.11 after I got reviews from Jason
> for most of the patches?
Absolutely, I intend to merge it.
--
MST
^ permalink raw reply
* Re: [PATCH 03/10] block: Revalidate i_bdev reference in bd_aquire()
From: Jan Kara @ 2017-02-09 15:54 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Tejun Heo, Dan Williams,
Thiago Jung Bauermann, NeilBrown, Jan Kara
In-Reply-To: <20170209124433.2626-4-jack@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 2107 bytes --]
On Thu 09-02-17 13:44:26, Jan Kara wrote:
> When a device gets removed, block device inode unhashed so that it is not
> used anymore (bdget() will not find it anymore). Later when a new device
> gets created with the same device number, we create new block device
> inode. However there may be file system device inodes whose i_bdev still
> points to the original block device inode and thus we get two active
> block device inodes for the same device. They will share the same
> gendisk so the only visible differences will be that page caches will
> not be coherent and BDIs will be different (the old block device inode
> still points to unregistered BDI).
>
> Fix the problem by checking in bd_acquire() whether i_bdev still points
> to active block device inode and re-lookup the block device if not. That
> way any open of a block device happening after the old device has been
> removed will get correct block device inode.
Thiago spotted a stupid bug in this patch (calling bd_forget() on bdev
instead of inode). Fixed version is attached.
Honza
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/block_dev.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 601b71b76d7f..360439373a66 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1043,13 +1043,22 @@ static struct block_device *bd_acquire(struct inode *inode)
>
> spin_lock(&bdev_lock);
> bdev = inode->i_bdev;
> - if (bdev) {
> + if (bdev && !inode_unhashed(bdev->bd_inode)) {
> bdgrab(bdev);
> spin_unlock(&bdev_lock);
> return bdev;
> }
> spin_unlock(&bdev_lock);
>
> + /*
> + * i_bdev references block device inode that was already shut down
> + * (corresponding device got removed). Remove the reference and look
> + * up block device inode again just in case new device got
> + * reestablished under the same device number.
> + */
> + if (bdev)
> + bd_forget(bdev);
> +
> bdev = bdget(inode->i_rdev);
> if (bdev) {
> spin_lock(&bdev_lock);
> --
> 2.10.2
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
[-- Attachment #2: 0003-block-Revalidate-i_bdev-reference-in-bd_aquire.patch --]
[-- Type: text/x-patch, Size: 2001 bytes --]
>From aaf612333753b948a96aebe4a2f8066ed45ef164 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Thu, 9 Feb 2017 12:16:30 +0100
Subject: [PATCH 03/10] block: Revalidate i_bdev reference in bd_aquire()
When a device gets removed, block device inode unhashed so that it is not
used anymore (bdget() will not find it anymore). Later when a new device
gets created with the same device number, we create new block device
inode. However there may be file system device inodes whose i_bdev still
points to the original block device inode and thus we get two active
block device inodes for the same device. They will share the same
gendisk so the only visible differences will be that page caches will
not be coherent and BDIs will be different (the old block device inode
still points to unregistered BDI).
Fix the problem by checking in bd_acquire() whether i_bdev still points
to active block device inode and re-lookup the block device if not. That
way any open of a block device happening after the old device has been
removed will get correct block device inode.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/block_dev.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 601b71b76d7f..68e855fdce58 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1043,13 +1043,22 @@ static struct block_device *bd_acquire(struct inode *inode)
spin_lock(&bdev_lock);
bdev = inode->i_bdev;
- if (bdev) {
+ if (bdev && !inode_unhashed(bdev->bd_inode)) {
bdgrab(bdev);
spin_unlock(&bdev_lock);
return bdev;
}
spin_unlock(&bdev_lock);
+ /*
+ * i_bdev references block device inode that was already shut down
+ * (corresponding device got removed). Remove the reference and look
+ * up block device inode again just in case new device got
+ * reestablished under the same device number.
+ */
+ if (bdev)
+ bd_forget(inode);
+
bdev = bdget(inode->i_rdev);
if (bdev) {
spin_lock(&bdev_lock);
--
2.10.2
^ permalink raw reply related
* Re: [PATCH 05/24] fs: Get proper reference for s_bdi
From: Jan Kara @ 2017-02-09 15:52 UTC (permalink / raw)
To: Boaz Harrosh; +Cc: Jan Kara, linux-fsdevel, Christoph Hellwig, linux-block
In-Reply-To: <d671b948-9449-b0df-27ac-79a768077a64@plexistor.com>
On Thu 09-02-17 16:36:13, Boaz Harrosh wrote:
> On 02/02/2017 07:34 PM, Jan Kara wrote:
> > So far we just relied on block device to hold a bdi reference for us
> > while the filesystem is mounted. While that works perfectly fine, it is
> > a bit awkward that we have a pointer to a refcounted structure in the
> > superblock without proper reference. So make s_bdi hold a proper
> > reference to block device's BDI. No filesystem using mount_bdev()
> > actually changes s_bdi so this is safe and will make bdev filesystems
> > work the same way as filesystems needing to set up their private bdi.
> >
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> > fs/super.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/super.c b/fs/super.c
> > index 31dc4c6450ef..dfb95ccd4351 100644
> > --- a/fs/super.c
> > +++ b/fs/super.c
> > @@ -1047,12 +1047,9 @@ static int set_bdev_super(struct super_block *s, void *data)
> > {
> > s->s_bdev = data;
> > s->s_dev = s->s_bdev->bd_dev;
> > + s->s_bdi = bdi_get(s->s_bdev->bd_bdi);
> > + s->s_iflags |= SB_I_DYNBDI;
> >
> > - /*
> > - * We set the bdi here to the queue backing, file systems can
> > - * overwrite this in ->fill_super()
> > - */
>
> Question: So I have an FS that uses mount_bdev but than goes and
> overrides sb->s_bdev in ->fill_super() anyway. This is because of two
> reasons. One because I have many more devices. (like btrfs I'm
> moulti-dev) but I like to use mount_bdev because of the somewhat delicate
> handling of automatic bind-mounts.
>
> For me it is a bigger hack to get the ref-counting and bind-mounts
> locking correctly then to bdi_put and say the new super_setup_bdi(sb) in
> fill_super. Would you expect problems?
No, that should work just fine.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox