RE: [PATCH] blk-mq: use after free when freeing tag sets
Oh, the same fix was applied from another developer:
https://git.kernel.org/cgit/linux/kernel/git/axboe/linux-block.git/commit/?h=for-linus&id=f42d79ab67322e51b92dd7aa965e310c71352a64
Yours came first, though I just missed it.
> On 10/02/2015 09:40 PM, Sasha Levin wrote:
> > Commit f26cdc853 ("blk-mq: Shared tag enhancements") has introduced a use
> > after
> > free where it tried to free the cpumask var out of the tag set but the tag
> > set
> > was already freed by blk_mq_free_rq_map().
> >
> > Signed-off-by: Sasha Levin
> > ---
> > block/blk-mq-tag.c |1 +
> > block/blk-mq.c |7 ++-
> > 2 files changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
> > index f6020c6..c03eeb8 100644
> > --- a/block/blk-mq-tag.c
> > +++ b/block/blk-mq-tag.c
> > @@ -628,6 +628,7 @@ void blk_mq_free_tags(struct blk_mq_tags *tags)
> > {
> > bt_free(&tags->bitmap_tags);
> > bt_free(&tags->breserved_tags);
> > + free_cpumask_var(tags->cpumask);
> > kfree(tags);
> > }
> >
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index 5692c15..197a615 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -2270,12 +2270,9 @@ void blk_mq_free_tag_set(struct blk_mq_tag_set *set)
> > {
> > int i;
> >
> > - for (i = 0; i < set->nr_hw_queues; i++) {
> > - if (set->tags[i]) {
> > + for (i = 0; i < set->nr_hw_queues; i++)
> > + if (set->tags[i])
> > blk_mq_free_rq_map(set, set->tags[i], i);
> > - free_cpumask_var(set->tags[i]->cpumask);
> > - }
> > - }
> >
> > kfree(set->tags);
> > set->tags = NULL;
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/7 v2] powerpc/dma-mapping: override dma_get_page_shift
On Tue, Oct 27, 2015 at 05:02:16PM +1100, Alexey Kardashevskiy wrote:
> >+unsigned long dma_get_page_shift(struct device *dev)
> >+{
> >+struct iommu_table *tbl = get_iommu_table_base(dev);
> >+if (tbl)
> >+return tbl->it_page_shift;
>
>
> All PCI devices have this initialized on POWER (at least, our, IBM's
> POWER) so 4K will always be returned here while in the case of
> (get_dma_ops(dev)==&dma_direct_ops) it could actually return
> PAGE_SHIFT. Is 4K still preferred value to return here?
4k is always a safe option to return, but ideally you want to return the
highest guaranteed DMA address alignment. The driver just needs to know
which bits to mask from virtual addresses such that the offset is the
same as the DMA address.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/5 v3] Fix NVMe driver support on Power with 32-bit DMA
On Tue, Oct 27, 2015 at 03:20:10PM -0700, Nishanth Aravamudan wrote: > On 26.10.2015 [18:27:46 -0700], David Miller wrote: > > From: Nishanth Aravamudan > > Date: Fri, 23 Oct 2015 13:54:20 -0700 > > > > > 1) add a generic dma_get_page_shift implementation that just returns > > > PAGE_SHIFT > > > > I won't object to this patch series, but if I had implemented this I > > would have required the architectures to implement this explicitly, > > one-by-one. I think it is less error prone and more likely to end > > up with all the architectures setting this correctly. > > Well, looks like I should spin up a v4 anyways for the powerpc changes. > So, to make sure I understand your point, should I make the generic > dma_get_page_shift a compile-error kind of thing? It will only fail on > architectures that actually build the NVME driver (as the only caller). > But I'm not sure how exactly to achieve that, if you could give a bit > more detail I'd appreciate it! If you're suggesting to compile-time break architectures that currently work just fine with NVMe, let me stop you right there. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/5 v3] Fix NVMe driver support on Power with 32-bit DMA
On Tue, Oct 27, 2015 at 05:54:43PM -0700, David Miller wrote: > From: "Busch, Keith" > Date: Tue, 27 Oct 2015 22:36:43 + > > > If you're suggesting to compile-time break architectures that currently > > work just fine with NVMe, let me stop you right there. > > Silently "working" without the architecture maintainer having to explicity > look at the new interface and make sure his platform is implementing it > properly is an extremely bad practice. It won't work if it's wrong. It'll BUG_ON, and I'll be assigned to help fix it, like what's happened here (on a private bugzilla). The "new" interface for all the other architectures is the same as the old one we've been using for the last 5 years. I welcome x86 maintainer feedback to confirm virtual and DMA addresses have the same offset at 4k alignment, but I have to insist we don't break my currently working hardware to force their attention. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/5 v3] Fix NVMe driver support on Power with 32-bit DMA
On Thu, Oct 29, 2015 at 08:57:01AM -0700, Nishanth Aravamudan wrote: > On 29.10.2015 [04:55:36 -0700], Christoph Hellwig wrote: > > We had a quick cht about this issue and I think we simply should > > default to a NVMe controler page size of 4k everywhere as that's the > > safe default. This is also what we do for RDMA Memory reigstrations and > > it works fine there for SRP and iSER. > > So, would that imply changing just the NVMe driver code rather than > adding the dma_page_shift API at all? What about > architectures that can support the larger page sizes? There is an > implied performance impact, at least, of shifting the IO size down. It is the safe option, but you're right that it might have a measurable performance impact (can you run an experiment?). Maybe we should just change the driver to always use MPSMIN for the moment in the interest of time, and you can flush out the new API before the next merge window. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] NVMe: do not touch sq door bell if nvmeq has been suspended
Does this ever happen? The queue should be stopped before the bar is unmapped. If that's insufficient to guard against this, we've another problem this patch does not cover. That command will just timeout since it was accepted by the driver, but not actually submitted to anything. The request needs to be requeued or return BLK_MQ_RQ_QUEUE_BUSY. > -Original Message- > From: Wenbo Wang [mailto:mail_weber_w...@163.com] > Sent: Monday, February 01, 2016 8:42 AM > To: ax...@fb.com; Busch, Keith > Cc: linux-kernel@vger.kernel.org; Wenbo Wang; Wenbo Wang; > linux-n...@lists.infradead.org > Subject: [PATCH] NVMe: do not touch sq door bell if nvmeq has been suspended > > If __nvme_submit_cmd races with nvme_dev_disable, nvmeq > could have been suspended and dev->bar could have been > unmapped. Do not touch sq door bell in this case. > > Signed-off-by: Wenbo Wang > Reviewed-by: Wenwei Tao > CC: linux-n...@lists.infradead.org > --- > drivers/nvme/host/pci.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c > index 8b1a725..2288712 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -325,7 +325,8 @@ static void __nvme_submit_cmd(struct nvme_queue *nvmeq, > > if (++tail == nvmeq->q_depth) > tail = 0; > - writel(tail, nvmeq->q_db); > + if (likely(nvmeq->cq_vector >= 0)) > + writel(tail, nvmeq->q_db); > nvmeq->sq_tail = tail; > } > > -- > 1.8.3.1
RE: Regression: Kernel unbootable since commit 4d6b4e6 - found by bisection
> Since early in the 4.3-rcx series, my Dell Latitude D600 with a 32-bit kernel > fails to boot. Unfortunately, I did not discover this until late in the > 4.4-rcX > cycle. The symptom is that the kernel echos the "Loading initial ramdisk ..." > message and then hangs. > > The problem was bisected to commit 4d6b4es" x86/PCI/ACPI: Use common interface > to support PCI host bridge" from Jiang Liu . The > bisection was tested by reverting the commit in question. I have not yet tried > to analyze the patch to see where it might be going wrong. > > Please suggest any trial prints to debug this issue. Does this fix help you? It is included with the most recent 4.4 rc. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=727ae8be30b428082d3519817f4fb98b712d457d -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
