Skip to contentSkip to conversation

Block fixes for 4.5-final

GIT PULL Jens Axboe 3 participants 8 messages Mar 3, 2016

lore
Share & tools
lore

Jens Axboe

author
#0 lore raw

Hi Linus,

A set of fixes for 4.5-rc6 - it's a lot bigger than I would like at this point, but there's really nothing in here that we should not merge for 4.5 final - with a possible exception being a few of the lightnvm fixes, that has a cleanup part before a fix, and the cgroup writeback enable for block devices from Shaohua (which we'd like to get into 4.5 final). The grunt of it should have gone in last week, but continued bug fixing for NVMe meant that it got pushed to this week.

In details, this pull request contains:

- Various fixes for lightnvm, from Alan, Javier, and Matias.

- Three bug fixes for xen blk-front/back from Jan Beulich, and Konrad.

- Extensive set of fixes for NVMe, fixing hot unplug problems and a regression in length of user commands supported. From Keith and Christoph, single fix from me.

- blk-mq potential oops fix in tag depth updates, from Keith.

- A fix for an umount with cgroup writeback regression in this series from Tejun.

- Enable block device cgroup writeback, from Shaohua. The grunt of the code for accounting was already there, this just enables it.

- Set of 4 patches for the block core on merging, from Ming Lei.

Please pull!

git://git.kernel.dk/linux-block.git for-linus

----------------------------------------------------------------
Alan (1):
lightnvm: fix up nonsensical configure overrun checking

Christoph Hellwig (4):
nvme: set queue limits for the admin queue
nvme: fix max_segments integer truncation
block: fix blk_rq_get_max_sectors for driver private requests
block: support large requests in blk_rq_map_user_iov

Jan Beulich (2):
xen-blkfront: rename indirect descriptor parameter
xen-blkback: advertise indirect segment support earlier

Javier González (2):
lightnvm: update closed list outside of intr context
lightnvm: generalize rrpc ppa calculations

Jens Axboe (2):
Merge branch 'stable/for-jens-4.5' of git://git.kernel.org/.../konrad/xen into for-linus
block: use cleaner variant of checking for block requests

Keith Busch (8):
blk-mq: Fix NULL pointer updating nr_requests
NVMe: Don't unmap controller registers on reset
NVMe: Use IDA for namespace disk naming
NVMe: Fix namespace removal deadlock
NVMe: Simplify device reset failure
NVMe: Move error handling to failed reset handler
NVMe: Don't allow unsupported flags
NVMe: Fix 0-length integrity payload

Konrad Rzeszutek Wilk (1):
xen/blback: Fit the important information of the thread in 17 characters

Matias Bjørling (3):

      lightnvm: fold get bb tbl when using dual/quad plane mode
      lightnvm: rename ->nr_pages to ->nr_sects
      lightnvm: remove struct nvm_dev->total_blocks

Ming Lei (4):
block: bio: introduce helpers to get the 1st and last bvec
block: check virt boundary in bio_will_gap()
block: get the 1st and last bvec via helpers
block: merge: get the 1st and last bvec via helpers

Shaohua Li (1):
block-dev: enable writeback cgroup support

Tejun Heo (1):
writeback: flush inode cgroup wb switches instead of pinning super_block

 block/blk-map.c                    |  91 ++++++++++++++--------
 block/blk-merge.c                  |   8 +-
 block/blk-mq.c                     |   2 +
 drivers/block/xen-blkback/xenbus.c |  20 ++---
 drivers/block/xen-blkfront.c       |   6 +-
 drivers/lightnvm/core.c            |  19 ++---
 drivers/lightnvm/gennvm.c          |   7 +-
 drivers/lightnvm/rrpc.c            |  98 +++++++++++++-----------
 drivers/lightnvm/rrpc.h            |  15 +++-
 drivers/nvme/host/core.c           | 111 +++++++++++++++++++--------
 drivers/nvme/host/lightnvm.c       |  46 ++++++++++--
 drivers/nvme/host/nvme.h           |   8 ++
 drivers/nvme/host/pci.c            | 149 ++++++++++++++++++++++---------------
 fs/block_dev.c                     |   6 +-
 fs/fs-writeback.c                  |  54 ++++++++++----
 fs/super.c                         |   1 +
 include/linux/bio.h                |  37 +++++++++
 include/linux/blkdev.h             |  25 +++++--
 include/linux/lightnvm.h           |   8 +-
 include/linux/writeback.h          |   5 ++
 20 files changed, 484 insertions(+), 232 deletions(-)
Signature · 1 line
Jens Axboe
#1 lore raw

On Thu, Mar 3, 2016 at 9:25 AM, Jens Axboe <[email protected]> wrote:
=>

Quoted 3 lines

A set of fixes for 4.5-rc6 - it's a lot bigger than I would like at this point, but there's really nothing in here that we should not merge for 4.5 final - with a possible exception [..]

With the possible exception of pretty much everything.

NONE of this seems really to be appropriate for this stage. It doesn't fix regressions, it doesn't fix security stuff, it doesn't really fix major oopses.

Why did you send it to me?

Right now, the block layer has been the problem child for several releases. This has got to stop.

Example of a couple of commits that made me decide to actually unpull this already after I had pulled it:

- commit 35b3ccc7d71c (from yesterday) changed old code that checked
for REQ_TYPE_BLOCK_PC to check for a bigger range

- then, today, commit 5b16f4f2b5e9 then changes that same code to
instead just check for REQ_TYPE_FS

Before this cluster-fuck of a pull request, that code had apparently worked well enough that it hadn't been touched since 2012.

So the "bug" it fixed clearly was clearly not hugely critical. The fix itself was clearly not discussed or thought out, since it ended up changing. It wasn't even important enough to mark for stable, although the code has clearly been that way for almost three years now.

And Gods, that was just the most obvious "this pull request is pure shit".

The rest of the pull request really in no way looked critical enough to be rc6+ material. Seriously.

So why the f*ck does the block layer end up being this kind of a problem?

Really. You need to get a grip, and start thinking about your pull requests a *lot* more.m None of this "let's send Linus random crap at any time in the release process".

I pulled it and then spent half an hour thinking about it, and decided that there's no way in hell pulling this is the right thing, so I had to not just unpull it, but undo and then re-do another pull I had done in the meantime.

Get your act together, Jens. You knew this was big and late. And absolutely *none* of it looked critical to me.

Feel free to resend the parts that are actually critical, but explain exactly why they are so critical when you do.

Linus

#2 lore raw
Quoted 3 lines · Linus Torvalds

Feel free to resend the parts that are actually critical, but explain exactly why they are so critical when you do.

Just to clarify: anything that contains "lightnvm" is definitely not critical enough. Are there even any users? I'm done seeing these constant large lightnvm patches.

NVMe is certainly a lot more important, but if it's about hot unplug that has never been reliable before, or about cleaning up the reset sequence by avoiding unmapping the register space, I really don't see what makes those critical patches that should go in outside of the merge window.

So no more of this "several hundred lines of random code changes after rc6 for issues that aren't fixing an actual critical bug and has never worked before either".

The block pull request was literally three times the size of all the other (*seven*) pull requests I've done today that were actually small fixes. Yeah, I'm not going to say that all of those were necessarily truly critical either, but at least they were very small and clearly fixes, and several of them fixed things like actual BUG_ON's or boot hangs.

Linus

author
#4 lore raw
In reply to Linus Torvalds
Quoted 8 lines · Linus Torvalds

On Thu, Mar 3, 2016 at 9:25 AM, Jens Axboe <[email protected]> wrote:
=>

Quoted 3 lines

A set of fixes for 4.5-rc6 - it's a lot bigger than I would like at this point, but there's really nothing in here that we should not merge for 4.5 final - with a possible exception [..]

With the possible exception of pretty much everything.

Quoted 8 lines

NONE of this seems really to be appropriate for this stage. It doesn't fix regressions, it doesn't fix security stuff, it doesn't really fix major oopses.

Why did you send it to me?

Right now, the block layer has been the problem child for several releases. This has got to stop.

I've been a pushover, largely driven by the NVMe changes. But that said, the changes are good. Maybe a smaller subset of them could have waited, but the majority should go in. And they have been well tested, mostly in batches outside of my tree.

Quoted 9 lines

Example of a couple of commits that made me decide to actually unpull this already after I had pulled it:

- commit 35b3ccc7d71c (from yesterday) changed old code that checked
for REQ_TYPE_BLOCK_PC to check for a bigger range

- then, today, commit 5b16f4f2b5e9 then changes that same code to
instead just check for REQ_TYPE_FS

Quoted 7 lines

Before this cluster-fuck of a pull request, that code had apparently worked well enough that it hadn't been touched since 2012.

So the "bug" it fixed clearly was clearly not hugely critical. The fix itself was clearly not discussed or thought out, since it ended up changing. It wasn't even important enough to mark for stable, although the code has clearly been that way for almost three years now.

It does fix a regression - the change is that NVMe now uses the block layer for these types of requests, and they don't have to adhere to the regular fs limits of sizing. Hence we broke real use cases, of (for instance) pulling logs off devices. Both of the referenced commits were added yesterday, not today. And they should have been folded, but I had already committed the first one. I don't think that should preclude doing it much cleaner than the first one.

Quoted 11 lines

And Gods, that was just the most obvious "this pull request is pure shit".

The rest of the pull request really in no way looked critical enough to be rc6+ material. Seriously.

So why the f*ck does the block layer end up being this kind of a problem?

Really. You need to get a grip, and start thinking about your pull requests a *lot* more.m None of this "let's send Linus random crap at any time in the release process".

Quoted 7 lines

I pulled it and then spent half an hour thinking about it, and decided that there's no way in hell pulling this is the right thing, so I had to not just unpull it, but undo and then re-do another pull I had done in the meantime.

Get your act together, Jens. You knew this was big and late. And absolutely *none* of it looked critical to me.

I realize it was big, and it is much bigger than I wanted. And I also realize that this has been a recurring theme the last couple of release. But that's not me being lazy, there's just been a larger amount of churn later in the cycle.

Quoted 2 lines

Feel free to resend the parts that are actually critical, but explain exactly why they are so critical when you do.

Fair enough, I can boil it down somewhat. But honestly, the only stuff I'd feel comfortable pulling out now would be the lightnvm changes which aren't that critical due to the user base, though that's also why it would be fine to shove it in now. And the cgroup writeback enable, which can wait. The two commits referenced above could be folded, but they'd still be in the new pull request.

So let me know if you want that, or we can proceed with the current branch, because most of it should really go in as-is.

Signature · 1 line
Jens Axboe
#3 lore raw
In reply to Linus Torvalds
Quoted 9 lines · Linus Torvalds
Quoted 3 lines · Linus Torvalds

Feel free to resend the parts that are actually critical, but explain exactly why they are so critical when you do.

Just to clarify: anything that contains "lightnvm" is definitely not critical enough. Are there even any users? I'm done seeing these constant large lightnvm patches.

Right, see previous email, that can surely be pulled. But at the same time, it's not exactly high impact stuff. That said, it should have gone to 4.6 originally.

Quoted 5 lines

NVMe is certainly a lot more important, but if it's about hot unplug that has never been reliable before, or about cleaning up the reset sequence by avoiding unmapping the register space, I really don't see what makes those critical patches that should go in outside of the merge window.

Fair enough.

Quoted 3 lines

So no more of this "several hundred lines of random code changes after rc6 for issues that aren't fixing an actual critical bug and has never worked before either".

I'll run a tighter ship for post-merge window.

Quoted 6 lines

The block pull request was literally three times the size of all the other (*seven*) pull requests I've done today that were actually small fixes. Yeah, I'm not going to say that all of those were necessarily truly critical either, but at least they were very small and clearly fixes, and several of them fixed things like actual BUG_ON's or boot hangs.

I could have split it in 3? :-)

Signature · 1 line
Jens Axboe
#5 lore raw
In reply to Jens Axboe
Quoted 8 lines · Jens Axboe

It does fix a regression - the change is that NVMe now uses the block layer for these types of requests, and they don't have to adhere to the regular fs limits of sizing. Hence we broke real use cases, of (for instance) pulling logs off devices. Both of the referenced commits were added yesterday, not today. And they should have been folded, but I had already committed the first one. I don't think that should preclude doing it much cleaner than the first one.

Why does this affect only NVMe, and not all the other drivers that have been around forever? What is that magical case that breaks? Details, please.

Quoted 9 lines

Fair enough, I can boil it down somewhat. But honestly, the only stuff I'd feel comfortable pulling out now would be the lightnvm changes which aren't that critical due to the user base, though that's also why it would be fine to shove it in now. And the cgroup writeback enable, which can wait. The two commits referenced above could be folded, but they'd still be in the new pull request.

So let me know if you want that, or we can proceed with the current branch, because most of it should really go in as-is.

I basically want for every commit an explanation of why it's so critical by now. I want to make you have to *think* and explain before you send stuff at this stage, and I want to understand why each commit is so important.

Because really, this has been going on far too long, and this pull request looked singularly pointless.

No way do I want things like cgroup writeback changes outside the merge window, for example, unless it's a major performance regression (with numbers) or something like that.

No way do I want any lightnvm stuff.

No way do I want big "cleanup" patches.

Linus

author
#6 lore raw
Quoted 10 lines · Linus Torvalds
Quoted 8 lines · Jens Axboe

It does fix a regression - the change is that NVMe now uses the block layer for these types of requests, and they don't have to adhere to the regular fs limits of sizing. Hence we broke real use cases, of (for instance) pulling logs off devices. Both of the referenced commits were added yesterday, not today. And they should have been folded, but I had already committed the first one. I don't think that should preclude doing it much cleaner than the first one.

Quoted 3 lines

Why does this affect only NVMe, and not all the other drivers that have been around forever? What is that magical case that breaks? Details, please.

Development around NVMe is a lot more active than any other driver. And that tends to drive a lot more testing, and find a lot of other bugs. That, and the fact that NVMe is still fairly young. On top of that, NVMe has been driving/utilizing some parts of blk-mq, and exercising things like surprise hot removal that haven't seen a ton of testing.

Quoted 18 lines
Quoted 9 lines

Fair enough, I can boil it down somewhat. But honestly, the only stuff I'd feel comfortable pulling out now would be the lightnvm changes which aren't that critical due to the user base, though that's also why it would be fine to shove it in now. And the cgroup writeback enable, which can wait. The two commits referenced above could be folded, but they'd still be in the new pull request.

So let me know if you want that, or we can proceed with the current branch, because most of it should really go in as-is.

I basically want for every commit an explanation of why it's so critical by now. I want to make you have to *think* and explain before you send stuff at this stage, and I want to understand why each commit is so important.

Because really, this has been going on far too long, and this pull request looked singularly pointless.

Quoted 7 lines

No way do I want things like cgroup writeback changes outside the merge window, for example, unless it's a major performance regression (with numbers) or something like that.

No way do I want any lightnvm stuff.

No way do I want big "cleanup" patches.

I'll boil it down.

Signature · 1 line
Jens Axboe
author
#7 lore raw
Quoted 9 lines · Jens Axboe
Quoted 7 lines

No way do I want things like cgroup writeback changes outside the merge window, for example, unless it's a major performance regression (with numbers) or something like that.

No way do I want any lightnvm stuff.

No way do I want big "cleanup" patches.

I'll boil it down.

This is the boiled down version. I'll run it through some testing, and if all goes well, send it out late tomorrow.

----------------------------------------------------------------
Christoph Hellwig (4):
nvme: set queue limits for the admin queue
nvme: fix max_segments integer truncation
block: fix blk_rq_get_max_sectors for driver private requests
block: support large requests in blk_rq_map_user_iov

Keith Busch (7):
NVMe: Don't unmap controller registers on reset
NVMe: Use IDA for namespace disk naming
NVMe: Fix namespace removal deadlock
NVMe: Simplify device reset failure
NVMe: Move error handling to failed reset handler
NVMe: Don't allow unsupported flags
NVMe: Fix 0-length integrity payload

Ming Lei (4):
block: bio: introduce helpers to get the 1st and last bvec
block: check virt boundary in bio_will_gap()
block: get the 1st and last bvec via helpers
block: merge: get the 1st and last bvec via helpers

Tejun Heo (1):
writeback: flush inode cgroup wb switches instead of pinning super_block

 block/blk-map.c           |  91 ++++++++++++++++++----------
 block/blk-merge.c         |   8 +--
 drivers/nvme/host/core.c  | 111 ++++++++++++++++++++++++----------
 drivers/nvme/host/nvme.h  |   8 +++
 drivers/nvme/host/pci.c   | 149 ++++++++++++++++++++++++++++------------------
 fs/fs-writeback.c         |  54 +++++++++++++----
 fs/super.c                |   1 +
 include/linux/bio.h       |  37 ++++++++++++
 include/linux/blkdev.h    |  25 +++++---
 include/linux/writeback.h |   5 ++
 10 files changed, 341 insertions(+), 148 deletions(-)
Signature · 1 line
Jens Axboe