LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [dm-devel] [PATCH 04/26] block: add a flag to make put_disk on partially initalized disks safer
From: Luis Chamberlain @ 2021-05-21 17:28 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: nvdimm, Ulf Hansson, Mike Snitzer, linux-m68k, linux-nvme,
	Song Liu, dm-devel, Joshua Morris, linux-s390, Dave Jiang,
	Maxim Levitsky, Vishal Verma, Christian Borntraeger,
	Geert Uytterhoeven, Matias Bjorling, Nitin Gupta, Vasily Gorbik,
	linux-xtensa, Alex Dubov, Heiko Carstens, Coly Li, linux-raid,
	linux-bcache, linux-block, drbd-dev, Philip Kelleher,
	Dan Williams, Jens Axboe, Chris Zankel, Max Filippov, linux-mmc,
	Philipp Reisner, Jim Paris, Minchan Kim, Lars Ellenberg,
	linuxppc-dev
In-Reply-To: <20210521055116.1053587-5-hch@lst.de>

On Fri, May 21, 2021 at 07:50:54AM +0200, Christoph Hellwig wrote:
> Add a flag to indicate that __device_add_disk did grab a queue reference
> so that disk_release only drops it if we actually had it.  This sort
> out one of the major pitfals with partially initialized gendisk that
> a lot of drivers did get wrong or still do.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis

^ permalink raw reply

* Re: [PATCH v2 01/11] powerpc: remove interrupt exit helpers unused argument
From: Christophe Leroy @ 2021-05-21 17:46 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20210521114422.3179350-2-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> a écrit :

> The msr argument is not used, remove it.

Why not keep and use that msr argument instead of re-reading regs->msr ?

>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/include/asm/asm-prototypes.h | 4 ++--
>  arch/powerpc/kernel/interrupt.c           | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/asm-prototypes.h  
> b/arch/powerpc/include/asm/asm-prototypes.h
> index 1c7b75834e04..95492655462e 100644
> --- a/arch/powerpc/include/asm/asm-prototypes.h
> +++ b/arch/powerpc/include/asm/asm-prototypes.h
> @@ -71,8 +71,8 @@ void __init machine_init(u64 dt_ptr);
>  #endif
>  long system_call_exception(long r3, long r4, long r5, long r6, long  
> r7, long r8, unsigned long r0, struct pt_regs *regs);
>  notrace unsigned long syscall_exit_prepare(unsigned long r3, struct  
> pt_regs *regs, long scv);
> -notrace unsigned long interrupt_exit_user_prepare(struct pt_regs  
> *regs, unsigned long msr);
> -notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs  
> *regs, unsigned long msr);
> +notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs);
> +notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs);
>
>  long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
>  		      u32 len_high, u32 len_low);
> diff --git a/arch/powerpc/kernel/interrupt.c  
> b/arch/powerpc/kernel/interrupt.c
> index e0938ba298f2..38ae7057d6c2 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -352,7 +352,7 @@ notrace unsigned long  
> syscall_exit_prepare(unsigned long r3,
>  	return ret;
>  }
>
> -notrace unsigned long interrupt_exit_user_prepare(struct pt_regs  
> *regs, unsigned long msr)
> +notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
>  {
>  	unsigned long ti_flags;
>  	unsigned long flags;
> @@ -431,7 +431,7 @@ notrace unsigned long  
> interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
>
>  void preempt_schedule_irq(void);
>
> -notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs  
> *regs, unsigned long msr)
> +notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
>  {
>  	unsigned long flags;
>  	unsigned long ret = 0;
> --
> 2.23.0



^ permalink raw reply

* Re: [dm-devel] [PATCH 05/26] block: add blk_alloc_disk and blk_cleanup_disk APIs
From: Luis Chamberlain @ 2021-05-21 17:44 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: nvdimm, Ulf Hansson, Mike Snitzer, linux-m68k, linux-nvme,
	Song Liu, dm-devel, Joshua Morris, linux-s390, Dave Jiang,
	Maxim Levitsky, Vishal Verma, Christian Borntraeger,
	Geert Uytterhoeven, Matias Bjorling, Nitin Gupta, Vasily Gorbik,
	linux-xtensa, Alex Dubov, Heiko Carstens, Coly Li, linux-raid,
	linux-bcache, linux-block, drbd-dev, Philip Kelleher,
	Dan Williams, Jens Axboe, Chris Zankel, Max Filippov, linux-mmc,
	Philipp Reisner, Jim Paris, Minchan Kim, Lars Ellenberg,
	linuxppc-dev
In-Reply-To: <20210521055116.1053587-6-hch@lst.de>

On Fri, May 21, 2021 at 07:50:55AM +0200, Christoph Hellwig wrote:
> Add two new APIs to allocate and free a gendisk including the
> request_queue for use with BIO based drivers.  This is to avoid
> boilerplate code in drivers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/genhd.c         | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/genhd.h | 22 ++++++++++++++++++++++
>  2 files changed, 57 insertions(+)
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index e4974af3d729..6d4ce962866d 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -1302,6 +1302,25 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)
>  }
>  EXPORT_SYMBOL(__alloc_disk_node);
>  
> +struct gendisk *__blk_alloc_disk(int node)
> +{
> +	struct request_queue *q;
> +	struct gendisk *disk;
> +
> +	q = blk_alloc_queue(node);
> +	if (!q)
> +		return NULL;
> +
> +	disk = __alloc_disk_node(0, node);
> +	if (!disk) {
> +		blk_cleanup_queue(q);
> +		return NULL;
> +	}
> +	disk->queue = q;
> +	return disk;
> +}
> +EXPORT_SYMBOL(__blk_alloc_disk);

Its not obvious to me why using this new API requires you then to
set minors explicitly to 1, and yet here underneath we see the minors
argument passed is 0.

Nor is it clear from the documentation.

  Luis

^ permalink raw reply

* Re: Linux powerpc new system call instruction and ABI
From: Matheus Castanho @ 2021-05-21 19:40 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: libc-alpha, musl, Dmitry V. Levin, linux-api, libc-dev,
	linuxppc-dev
In-Reply-To: <1621400263.gf0mbqhkrf.astroid@bobo.none>


Nicholas Piggin <npiggin@gmail.com> writes:

> Excerpts from Nicholas Piggin's message of May 19, 2021 12:50 pm:
>> Excerpts from Dmitry V. Levin's message of May 19, 2021 9:13 am:
>>> Hi,
>>>
>>> On Thu, Jun 11, 2020 at 06:12:01PM +1000, Nicholas Piggin wrote:
>>> [...]
>>>> - Error handling: The consensus among kernel, glibc, and musl is to move to
>>>>   using negative return values in r3 rather than CR0[SO]=1 to indicate error,
>>>>   which matches most other architectures, and is closer to a function call.
>>>
>>> Apparently, the patchset merged by commit v5.9-rc1~100^2~164 was
>>> incomplete: all functions defined in arch/powerpc/include/asm/ptrace.h and
>>> arch/powerpc/include/asm/syscall.h that use ccr are broken when scv is used.
>>> This includes syscall_get_error() and all its users including
>>> PTRACE_GET_SYSCALL_INFO API, which in turn makes strace unusable
>>> when scv is used.
>>>
>>> See also https://bugzilla.redhat.com/1929836
>>
>> I see, thanks. Using latest strace from github.com, the attached kernel
>> patch makes strace -k check results a lot greener.
>>
>> Some of the remaining failing tests look like this (I didn't look at all
>> of them yet):
>>
>> signal(SIGUSR1, 0xfacefeeddeadbeef)     = 0 (SIG_DFL)
>> write(1, "signal(SIGUSR1, 0xfacefeeddeadbe"..., 50signal(SIGUSR1, 0xfacefeeddeadbeef) = 0 (SIG_DFL)
>> ) = 50
>> signal(SIGUSR1, SIG_IGN)                = 0xfacefeeddeadbeef
>> write(2, "errno2name.c:461: unknown errno "..., 41errno2name.c:461: unknown errno 559038737) = 41
>> write(2, ": Unknown error 559038737\n", 26: Unknown error 559038737
>> ) = 26
>> exit_group(1)                           = ?
>>
>> I think the problem is glibc testing for -ve, but it should be comparing
>> against -4095 (+cc Matheus)
>>
>>   #define RET_SCV \
>>       cmpdi r3,0; \
>>       bgelr+; \
>>       neg r3,r3;
>
> This glibc patch at least gets that signal test working. Haven't run the
> full suite yet because of trouble making it work with a local glibc
> install...
>
> Thanks,
> Nick
>
> ---
>
> diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
> index c57bb1c05d..1ea4c3b917 100644
> --- a/sysdeps/powerpc/powerpc64/sysdep.h
> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
> @@ -398,8 +398,9 @@ LT_LABELSUFFIX(name,_name_end): ; \
>  #endif
>
>  #define RET_SCV \
> -    cmpdi r3,0; \
> -    bgelr+; \
> +    li r9,-4095; \
> +    cmpld r3,r9; \
> +    bltlr+; \
>      neg r3,r3;
>
>  #define RET_SC \

Hi Nick,

I agree the current code is accepting more values as errors than it
should. This change looks good to me. All glibc tests are passing with
it. I also built strace and checked one of the failing tests against a
glibc with your patch:

~/src/strace/tests$ uname -r
5.10.16-1-default

~/src/strace/tests$ /lib64/libc.so.6
GNU C Library (GNU libc) release release version 2.33 (git 9826b03b74).
[...]

~/src/strace/tests$ ./signal.gen.test
errno2name.c:461: unknown errno 559038737: Unknown error 559038737
signal.gen.test: failed test: ../signal failed with code 1

~/src/strace/tests$ ./signal
signal(SIGUSR1, SIG_IGN) = 0 (SIG_DFL)
signal(SIGUSR1, SIG_DFL) = 0x1 (SIG_IGN)
signal(SIGUSR1, 0xfacefeeddeadbeef) = 0 (SIG_DFL)
errno2name.c:461: unknown errno 559038737: Unknown error 559038737


Running with glibc containing the patch:

~/src/strace/tests$ ~/build/glibc/testrun.sh ./signal
signal(SIGUSR1, SIG_IGN) = 0 (SIG_DFL)
signal(SIGUSR1, SIG_DFL) = 0x1 (SIG_IGN)
signal(SIGUSR1, 0xfacefeeddeadbeef) = 0 (SIG_DFL)
signal(SIGUSR1, SIG_IGN) = 0xfacefeeddeadbeef
signal(-559038737, SIG_IGN) = -1 EINVAL (Invalid argument)
+++ exited with 0 +++



If the patch below looks OK to you and no one objects, I'll commit it to
glibc on Monday.

Thanks,
Matheus Castanho

---

From: Nicholas Piggin <npiggin@gmail.com>
Subject: [PATCH 1/1] powerpc: Fix handling of scv return error codes

When using scv on templated ASM syscalls, current code interprets any
negative return value as error, but the only valid error codes are in
the range -4095..-1 according to the ABI.

Reviewed-by: Matheus Castanho <msc@linux.ibm.com>
---
 sysdeps/powerpc/powerpc64/sysdep.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
index c57bb1c05d..1ea4c3b917 100644
--- a/sysdeps/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/powerpc/powerpc64/sysdep.h
@@ -398,8 +398,9 @@ LT_LABELSUFFIX(name,_name_end): ; \
 #endif

 #define RET_SCV \
-    cmpdi r3,0; \
-    bgelr+; \
+    li r9,-4095; \
+    cmpld r3,r9; \
+    bltlr+; \
     neg r3,r3;

 #define RET_SC \
--
2.31.1

^ permalink raw reply related

* Re: Linux powerpc new system call instruction and ABI
From: Florian Weimer @ 2021-05-21 19:52 UTC (permalink / raw)
  To: Matheus Castanho via Libc-alpha
  Cc: Matheus Castanho, linux-api, musl, Nicholas Piggin,
	Dmitry V. Levin, libc-dev, linuxppc-dev
In-Reply-To: <87a6oo4312.fsf@linux.ibm.com>

* Matheus Castanho via Libc-alpha:

> From: Nicholas Piggin <npiggin@gmail.com>
> Subject: [PATCH 1/1] powerpc: Fix handling of scv return error codes
>
> When using scv on templated ASM syscalls, current code interprets any
> negative return value as error, but the only valid error codes are in
> the range -4095..-1 according to the ABI.
>
> Reviewed-by: Matheus Castanho <msc@linux.ibm.com>

Please reference bug 27892 in the commit message.  I'd also appreciate a
backport to the 2.33 release branch (where you need to add NEWS manually
to add the bug reference).

Thanks,
Florian


^ permalink raw reply

* Re: Linux powerpc new system call instruction and ABI
From: Matheus Castanho @ 2021-05-21 20:00 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Matheus Castanho via Libc-alpha, linux-api, musl, Nicholas Piggin,
	Dmitry V. Levin, libc-dev, linuxppc-dev
In-Reply-To: <87k0nr6ezw.fsf@oldenburg.str.redhat.com>


Florian Weimer <fweimer@redhat.com> writes:

> * Matheus Castanho via Libc-alpha:
>
>> From: Nicholas Piggin <npiggin@gmail.com>
>> Subject: [PATCH 1/1] powerpc: Fix handling of scv return error codes
>>
>> When using scv on templated ASM syscalls, current code interprets any
>> negative return value as error, but the only valid error codes are in
>> the range -4095..-1 according to the ABI.
>>
>> Reviewed-by: Matheus Castanho <msc@linux.ibm.com>
>
> Please reference bug 27892 in the commit message.  I'd also appreciate a
> backport to the 2.33 release branch (where you need to add NEWS manually
> to add the bug reference).

No problem. [BZ #27892] appended to the commit title. I'll make sure to
backport to 2.33 as well.

Thanks

--
Matheus Castanho

^ permalink raw reply

* Re: Linux powerpc new system call instruction and ABI
From: Dmitry V. Levin @ 2021-05-21 20:52 UTC (permalink / raw)
  To: Matheus Castanho
  Cc: Florian Weimer, Matheus Castanho via Libc-alpha, linux-api, musl,
	Nicholas Piggin, libc-dev, linuxppc-dev
In-Reply-To: <87eedz3li3.fsf@linux.ibm.com>

On Fri, May 21, 2021 at 05:00:36PM -0300, Matheus Castanho wrote:
> Florian Weimer <fweimer@redhat.com> writes:
> > * Matheus Castanho via Libc-alpha:
> >> From: Nicholas Piggin <npiggin@gmail.com>
> >> Subject: [PATCH 1/1] powerpc: Fix handling of scv return error codes
> >>
> >> When using scv on templated ASM syscalls, current code interprets any
> >> negative return value as error, but the only valid error codes are in
> >> the range -4095..-1 according to the ABI.
> >>
> >> Reviewed-by: Matheus Castanho <msc@linux.ibm.com>
> >
> > Please reference bug 27892 in the commit message.  I'd also appreciate a
> > backport to the 2.33 release branch (where you need to add NEWS manually
> > to add the bug reference).
> 
> No problem. [BZ #27892] appended to the commit title. I'll make sure to
> backport to 2.33 as well.

Could you also mention in the commit message that the change fixes
'signal.gen.test' strace test where it was observed initially?


-- 
ldv

^ permalink raw reply

* Re: [PATCH 0/8] xen: harden frontends against malicious backends
From: Marek Marczykowski-Górecki @ 2021-05-21 10:43 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Jens Axboe, Stefano Stabellini, Jiri Slaby, Konrad Rzeszutek Wilk,
	netdev, linux-kernel, linux-block, Greg Kroah-Hartman,
	Jakub Kicinski, xen-devel, Boris Ostrovsky, linuxppc-dev,
	David S. Miller, Roger Pau Monné
In-Reply-To: <20210513100302.22027-1-jgross@suse.com>

[-- Attachment #1: Type: text/plain, Size: 2052 bytes --]

On Thu, May 13, 2021 at 12:02:54PM +0200, Juergen Gross wrote:
> Xen backends of para-virtualized devices can live in dom0 kernel, dom0
> user land, or in a driver domain. This means that a backend might
> reside in a less trusted environment than the Xen core components, so
> a backend should not be able to do harm to a Xen guest (it can still
> mess up I/O data, but it shouldn't be able to e.g. crash a guest by
> other means or cause a privilege escalation in the guest).
> 
> Unfortunately many frontends in the Linux kernel are fully trusting
> their respective backends. This series is starting to fix the most
> important frontends: console, disk and network.
> 
> It was discussed to handle this as a security problem, but the topic
> was discussed in public before, so it isn't a real secret.

Is it based on patches we ship in Qubes[1] and also I've sent here some
years ago[2]? I see a lot of similarities. If not, you may want to
compare them.

[1] https://github.com/QubesOS/qubes-linux-kernel/
[2] https://lists.xenproject.org/archives/html/xen-devel/2018-04/msg02336.html


> Juergen Gross (8):
>   xen: sync include/xen/interface/io/ring.h with Xen's newest version
>   xen/blkfront: read response from backend only once
>   xen/blkfront: don't take local copy of a request from the ring page
>   xen/blkfront: don't trust the backend response data blindly
>   xen/netfront: read response from backend only once
>   xen/netfront: don't read data from request on the ring page
>   xen/netfront: don't trust the backend response data blindly
>   xen/hvc: replace BUG_ON() with negative return value
> 
>  drivers/block/xen-blkfront.c    | 118 +++++++++-----
>  drivers/net/xen-netfront.c      | 184 ++++++++++++++-------
>  drivers/tty/hvc/hvc_xen.c       |  15 +-
>  include/xen/interface/io/ring.h | 278 ++++++++++++++++++--------------
>  4 files changed, 369 insertions(+), 226 deletions(-)
> 
> -- 
> 2.26.2
> 
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 01/26] block: refactor device number setup in __device_add_disk
From: Hannes Reinecke @ 2021-05-23  7:46 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Joshua Morris, Philip Kelleher, Minchan Kim, Nitin Gupta,
	Matias Bjorling, Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky,
	Alex Dubov, Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-2-hch@lst.de>

On 5/21/21 7:50 AM, Christoph Hellwig wrote:
> Untangle the mess around blk_alloc_devt by moving the check for
> the used allocation scheme into the callers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   block/blk.h             |  4 +-
>   block/genhd.c           | 96 ++++++++++++++++-------------------------
>   block/partitions/core.c | 15 +++++--
>   3 files changed, 49 insertions(+), 66 deletions(-)
> 
... and also fixes an issue with GENHD_FL_UP remained set in an error 
path in __device_add_disk().

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 02/26] block: move the DISK_MAX_PARTS sanity check into __device_add_disk
From: Hannes Reinecke @ 2021-05-23  7:48 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-3-hch@lst.de>

On 5/21/21 7:50 AM, Christoph Hellwig wrote:
> Keep this together with the first place that actually looks at
> ->minors and prepare for not passing a minors argument to
> alloc_disk.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   block/genhd.c | 13 ++++++-------
>   1 file changed, 6 insertions(+), 7 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 03/26] block: automatically enable GENHD_FL_EXT_DEVT
From: Hannes Reinecke @ 2021-05-23  7:50 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-4-hch@lst.de>

On 5/21/21 7:50 AM, Christoph Hellwig wrote:
> Automatically set the GENHD_FL_EXT_DEVT flag for all disks allocated
> without an explicit number of minors.  This is what all new block
> drivers should do, so make sure it is the default without boilerplate
> code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   block/genhd.c                    | 2 +-
>   block/partitions/core.c          | 4 ----
>   drivers/block/n64cart.c          | 2 +-
>   drivers/lightnvm/core.c          | 1 -
>   drivers/memstick/core/ms_block.c | 1 -
>   drivers/nvdimm/blk.c             | 1 -
>   drivers/nvdimm/btt.c             | 1 -
>   drivers/nvdimm/pmem.c            | 1 -
>   drivers/nvme/host/core.c         | 1 -
>   drivers/nvme/host/multipath.c    | 1 -
>   10 files changed, 2 insertions(+), 13 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 04/26] block: add a flag to make put_disk on partially initalized disks safer
From: Hannes Reinecke @ 2021-05-23  7:54 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-5-hch@lst.de>

On 5/21/21 7:50 AM, Christoph Hellwig wrote:
> Add a flag to indicate that __device_add_disk did grab a queue reference
> so that disk_release only drops it if we actually had it.  This sort
> out one of the major pitfals with partially initialized gendisk that
                        pitfalls

> a lot of drivers did get wrong or still do.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   block/genhd.c         | 7 +++++--
>   include/linux/genhd.h | 1 +
>   2 files changed, 6 insertions(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 05/26] block: add blk_alloc_disk and blk_cleanup_disk APIs
From: Hannes Reinecke @ 2021-05-23  7:55 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Joshua Morris, Philip Kelleher, Minchan Kim, Nitin Gupta,
	Matias Bjorling, Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky,
	Alex Dubov, Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-6-hch@lst.de>

On 5/21/21 7:50 AM, Christoph Hellwig wrote:
> Add two new APIs to allocate and free a gendisk including the
> request_queue for use with BIO based drivers.  This is to avoid
> boilerplate code in drivers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   block/genhd.c         | 35 +++++++++++++++++++++++++++++++++++
>   include/linux/genhd.h | 22 ++++++++++++++++++++++
>   2 files changed, 57 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 06/26] brd: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  7:58 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Joshua Morris, Philip Kelleher, Minchan Kim, Nitin Gupta,
	Matias Bjorling, Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky,
	Alex Dubov, Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-7-hch@lst.de>

On 5/21/21 7:50 AM, Christoph Hellwig wrote:
> Convert the brd driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.  This also
> allows to remove the request_queue pointer in struct request_queue,
> and to simplify the initialization as blk_cleanup_disk can be called
> on any disk returned from blk_alloc_disk.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/block/brd.c | 94 ++++++++++++++++-----------------------------
>   1 file changed, 33 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/block/brd.c b/drivers/block/brd.c
> index 7562cf30b14e..95694113e38e 100644
> --- a/drivers/block/brd.c
> +++ b/drivers/block/brd.c
> @@ -38,9 +38,7 @@
>    * device).
>    */
>   struct brd_device {
> -	int		brd_number;
> -
> -	struct request_queue	*brd_queue;
> +	int			brd_number;
>   	struct gendisk		*brd_disk;
>   	struct list_head	brd_list;
>   
> @@ -372,7 +370,7 @@ static LIST_HEAD(brd_devices);
>   static DEFINE_MUTEX(brd_devices_mutex);
>   static struct dentry *brd_debugfs_dir;
>   
> -static struct brd_device *brd_alloc(int i)
> +static int brd_alloc(int i)
>   {
>   	struct brd_device *brd;
>   	struct gendisk *disk;
> @@ -380,64 +378,55 @@ static struct brd_device *brd_alloc(int i)
>   
>   	brd = kzalloc(sizeof(*brd), GFP_KERNEL);
>   	if (!brd)
> -		goto out;
> +		return -ENOMEM;
>   	brd->brd_number		= i;
>   	spin_lock_init(&brd->brd_lock);
>   	INIT_RADIX_TREE(&brd->brd_pages, GFP_ATOMIC);
>   
> -	brd->brd_queue = blk_alloc_queue(NUMA_NO_NODE);
> -	if (!brd->brd_queue)
> -		goto out_free_dev;
> -
>   	snprintf(buf, DISK_NAME_LEN, "ram%d", i);
>   	if (!IS_ERR_OR_NULL(brd_debugfs_dir))
>   		debugfs_create_u64(buf, 0444, brd_debugfs_dir,
>   				&brd->brd_nr_pages);
>   
> -	/* This is so fdisk will align partitions on 4k, because of
> -	 * direct_access API needing 4k alignment, returning a PFN
> -	 * (This is only a problem on very small devices <= 4M,
> -	 *  otherwise fdisk will align on 1M. Regardless this call
> -	 *  is harmless)
> -	 */
> -	blk_queue_physical_block_size(brd->brd_queue, PAGE_SIZE);
> -	disk = brd->brd_disk = alloc_disk(max_part);
> +	disk = brd->brd_disk = blk_alloc_disk(NUMA_NO_NODE);
>   	if (!disk)
> -		goto out_free_queue;
> +		goto out_free_dev;
> +
>   	disk->major		= RAMDISK_MAJOR;
>   	disk->first_minor	= i * max_part;
> +	disk->minors		= max_part;
>   	disk->fops		= &brd_fops;
>   	disk->private_data	= brd;
>   	disk->flags		= GENHD_FL_EXT_DEVT;
>   	strlcpy(disk->disk_name, buf, DISK_NAME_LEN);
>   	set_capacity(disk, rd_size * 2);
> +	
> +	/*
> +	 * This is so fdisk will align partitions on 4k, because of
> +	 * direct_access API needing 4k alignment, returning a PFN
> +	 * (This is only a problem on very small devices <= 4M,
> +	 *  otherwise fdisk will align on 1M. Regardless this call
> +	 *  is harmless)
> +	 */
> +	blk_queue_physical_block_size(disk->queue, PAGE_SIZE);
>   

Maybe converting the comment to refer to 'PAGE_SIZE' instead of 4k while 
you're at it ...

>   	/* Tell the block layer that this is not a rotational device */
> -	blk_queue_flag_set(QUEUE_FLAG_NONROT, brd->brd_queue);
> -	blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, brd->brd_queue);
> +	blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
> +	blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, disk->queue);
> +	add_disk(disk);
> +	list_add_tail(&brd->brd_list, &brd_devices);
>   
> -	return brd;
> +	return 0;
>   
> -out_free_queue:
> -	blk_cleanup_queue(brd->brd_queue);
>   out_free_dev:
>   	kfree(brd);
> -out:
> -	return NULL;
> -}
> -
> -static void brd_free(struct brd_device *brd)
> -{
> -	put_disk(brd->brd_disk);
> -	blk_cleanup_queue(brd->brd_queue);
> -	brd_free_pages(brd);
> -	kfree(brd);
> +	return -ENOMEM;
>   }
>   
>   static void brd_probe(dev_t dev)
>   {
> -	struct brd_device *brd;
>   	int i = MINOR(dev) / max_part;
> +	struct brd_device *brd;
>   
>   	mutex_lock(&brd_devices_mutex);
>   	list_for_each_entry(brd, &brd_devices, brd_list) {
> @@ -445,13 +434,7 @@ static void brd_probe(dev_t dev)
>   			goto out_unlock;
>   	}
>   
> -	brd = brd_alloc(i);
> -	if (brd) {
> -		brd->brd_disk->queue = brd->brd_queue;
> -		add_disk(brd->brd_disk);
> -		list_add_tail(&brd->brd_list, &brd_devices);
> -	}
> -
> +	brd_alloc(i);
>   out_unlock:
>   	mutex_unlock(&brd_devices_mutex);
>   }
> @@ -460,7 +443,9 @@ static void brd_del_one(struct brd_device *brd)
>   {
>   	list_del(&brd->brd_list);
>   	del_gendisk(brd->brd_disk);
> -	brd_free(brd);
> +	blk_cleanup_disk(brd->brd_disk);
> +	brd_free_pages(brd);
> +	kfree(brd);
>   }
>   
>   static inline void brd_check_and_reset_par(void)
> @@ -485,7 +470,7 @@ static inline void brd_check_and_reset_par(void)
>   static int __init brd_init(void)
>   {
>   	struct brd_device *brd, *next;
> -	int i;
> +	int err, i;
>   
>   	/*
>   	 * brd module now has a feature to instantiate underlying device
> @@ -511,22 +496,11 @@ static int __init brd_init(void)
>   
>   	mutex_lock(&brd_devices_mutex);
>   	for (i = 0; i < rd_nr; i++) {
> -		brd = brd_alloc(i);
> -		if (!brd)
> +		err = brd_alloc(i);
> +		if (err)
>   			goto out_free;
> -		list_add_tail(&brd->brd_list, &brd_devices);
>   	}
>   
> -	/* point of no return */
> -
> -	list_for_each_entry(brd, &brd_devices, brd_list) {
> -		/*
> -		 * associate with queue just before adding disk for
> -		 * avoiding to mess up failure path
> -		 */
> -		brd->brd_disk->queue = brd->brd_queue;
> -		add_disk(brd->brd_disk);
> -	}
>   	mutex_unlock(&brd_devices_mutex);
>   
>   	pr_info("brd: module loaded\n");
> @@ -535,15 +509,13 @@ static int __init brd_init(void)
>   out_free:
>   	debugfs_remove_recursive(brd_debugfs_dir);
>   
> -	list_for_each_entry_safe(brd, next, &brd_devices, brd_list) {
> -		list_del(&brd->brd_list);
> -		brd_free(brd);
> -	}
> +	list_for_each_entry_safe(brd, next, &brd_devices, brd_list)
> +		brd_del_one(brd);
>   	mutex_unlock(&brd_devices_mutex);
>   	unregister_blkdev(RAMDISK_MAJOR, "ramdisk");
>   
>   	pr_info("brd: module NOT loaded !!!\n");
> -	return -ENOMEM;
> +	return err;
>   }
>   
>   static void __exit brd_exit(void)
> 
Other than that:

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 07/26] drbd: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  7:59 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-8-hch@lst.de>

On 5/21/21 7:50 AM, Christoph Hellwig wrote:
> Convert the drbd driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/block/drbd/drbd_main.c | 23 ++++++++---------------
>   1 file changed, 8 insertions(+), 15 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 08/26] pktcdvd: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  8:00 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-9-hch@lst.de>

On 5/21/21 7:50 AM, Christoph Hellwig wrote:
> Convert the pktcdvd driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/block/pktcdvd.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* [PATCH net-next] ethernet: ucc_geth: Use kmemdup() rather than kmalloc+memcpy
From: YueHaibing @ 2021-05-23  7:56 UTC (permalink / raw)
  To: leoyang.li, davem, kuba, rasmus.villemoes
  Cc: netdev, YueHaibing, linuxppc-dev, linux-kernel

Issue identified with Coccinelle.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/freescale/ucc_geth.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index e0936510fa34..51206272cc25 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3590,10 +3590,10 @@ static int ucc_geth_probe(struct platform_device* ofdev)
 	if ((ucc_num < 0) || (ucc_num > 7))
 		return -ENODEV;
 
-	ug_info = kmalloc(sizeof(*ug_info), GFP_KERNEL);
+	ug_info = kmemdup(&ugeth_primary_info, sizeof(*ug_info),
+			  GFP_KERNEL);
 	if (ug_info == NULL)
 		return -ENOMEM;
-	memcpy(ug_info, &ugeth_primary_info, sizeof(*ug_info));
 
 	ug_info->uf_info.ucc_num = ucc_num;
 
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH 09/26] rsxx: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  8:01 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-10-hch@lst.de>

On 5/21/21 7:50 AM, Christoph Hellwig wrote:
> Convert the rsxx driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/block/rsxx/dev.c       | 39 +++++++++++++---------------------
>   drivers/block/rsxx/rsxx_priv.h |  1 -
>   2 files changed, 15 insertions(+), 25 deletions(-)
> Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 10/26] zram: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  8:01 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Joshua Morris, Philip Kelleher, Minchan Kim, Nitin Gupta,
	Matias Bjorling, Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky,
	Alex Dubov, Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-11-hch@lst.de>

On 5/21/21 7:51 AM, Christoph Hellwig wrote:
> Convert the zram driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/block/zram/zram_drv.c | 19 ++++---------------
>   1 file changed, 4 insertions(+), 15 deletions(-)
> Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 11/26] lightnvm: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  8:02 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-12-hch@lst.de>

On 5/21/21 7:51 AM, Christoph Hellwig wrote:
> Convert the lightnvm driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/lightnvm/core.c | 23 +++++------------------
>   1 file changed, 5 insertions(+), 18 deletions(-)
> Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 12/26] bcache: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  8:04 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-13-hch@lst.de>

On 5/21/21 7:51 AM, Christoph Hellwig wrote:
> Convert the bcache driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/bcache/super.c | 15 ++++-----------
>   1 file changed, 4 insertions(+), 11 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 13/26] dm: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  8:10 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-14-hch@lst.de>

On 5/21/21 7:51 AM, Christoph Hellwig wrote:
> Convert the dm driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/dm.c | 16 +++++++---------
>   1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index ca2aedd8ee7d..3c7c2d257018 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1801,13 +1801,13 @@ static void cleanup_mapped_device(struct mapped_device *md)
>   		md->disk->private_data = NULL;
>   		spin_unlock(&_minor_lock);
>   		del_gendisk(md->disk);
> -		put_disk(md->disk);
>   	}
>   
> -	if (md->queue) {
> +	if (md->queue)
>   		dm_queue_destroy_keyslot_manager(md->queue);
> -		blk_cleanup_queue(md->queue);
> -	}
> +
> +	if (md->disk)
> +		blk_cleanup_disk(md->disk);
>   
>   	cleanup_srcu_struct(&md->io_barrier);
>   

Can't these conditionals be merged into a single 'if (md->disk)'?
Eg like:

	if (md->disk) {
		spin_lock(&_minor_lock);
		md->disk->private_data = NULL;
		spin_unlock(&_minor_lock);
		del_gendisk(md->disk);
		dm_queue_destroy_keyslot_manager(md->queue);
		blk_cleanup_disk(md->queue);
	}

We're now always allocating 'md->disk' and 'md->queue' together,
so how can we end up in a situation where one is set without the other?

> @@ -1869,13 +1869,10 @@ static struct mapped_device *alloc_dev(int minor)
>   	 * established. If request-based table is loaded: blk-mq will
>   	 * override accordingly.
>   	 */
> -	md->queue = blk_alloc_queue(numa_node_id);
> -	if (!md->queue)
> -		goto bad;
> -
> -	md->disk = alloc_disk_node(1, md->numa_node_id);
> +	md->disk = blk_alloc_disk(md->numa_node_id);
>   	if (!md->disk)
>   		goto bad;
> +	md->queue = md->disk->queue;
>   
>   	init_waitqueue_head(&md->wait);
>   	INIT_WORK(&md->work, dm_wq_work);
> @@ -1888,6 +1885,7 @@ static struct mapped_device *alloc_dev(int minor)
>   
>   	md->disk->major = _major;
>   	md->disk->first_minor = minor;
> +	md->disk->minors = 1;
>   	md->disk->fops = &dm_blk_dops;
>   	md->disk->queue = md->queue;
>   	md->disk->private_data = md;
> 
Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 14/26] md: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  8:12 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Joshua Morris, Philip Kelleher, Minchan Kim, Nitin Gupta,
	Matias Bjorling, Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky,
	Alex Dubov, Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-15-hch@lst.de>

On 5/21/21 7:51 AM, Christoph Hellwig wrote:
> Convert the md driver to use the blk_alloc_disk and blk_cleanup_disk
> helpers to simplify gendisk and request_queue allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/md/md.c | 25 +++++++++----------------
>   1 file changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 49f897fbb89b..d806be8cc210 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -5598,12 +5598,10 @@ static void md_free(struct kobject *ko)
>   	if (mddev->sysfs_level)
>   		sysfs_put(mddev->sysfs_level);
>   
> -	if (mddev->gendisk)
> +	if (mddev->gendisk) {
>   		del_gendisk(mddev->gendisk);
> -	if (mddev->queue)
> -		blk_cleanup_queue(mddev->queue);
> -	if (mddev->gendisk)
> -		put_disk(mddev->gendisk);
> +		blk_cleanup_disk(mddev->gendisk);
> +	}
>   	percpu_ref_exit(&mddev->writes_pending);
>   
>   	bioset_exit(&mddev->bio_set);
> @@ -5711,20 +5709,13 @@ static int md_alloc(dev_t dev, char *name)
>   		goto abort;
>   
>   	error = -ENOMEM;
> -	mddev->queue = blk_alloc_queue(NUMA_NO_NODE);
> -	if (!mddev->queue)
> +	disk = blk_alloc_disk(NUMA_NO_NODE);
> +	if (!disk)
>   		goto abort;
>   
> -	blk_set_stacking_limits(&mddev->queue->limits);
> -
> -	disk = alloc_disk(1 << shift);
> -	if (!disk) {
> -		blk_cleanup_queue(mddev->queue);
> -		mddev->queue = NULL;
> -		goto abort;
> -	}
>   	disk->major = MAJOR(mddev->unit);
>   	disk->first_minor = unit << shift;
> +	disk->minors = 1 << shift;
>   	if (name)
>   		strcpy(disk->disk_name, name);
>   	else if (partitioned)
> @@ -5733,7 +5724,9 @@ static int md_alloc(dev_t dev, char *name)
>   		sprintf(disk->disk_name, "md%d", unit);
>   	disk->fops = &md_fops;
>   	disk->private_data = mddev;
> -	disk->queue = mddev->queue;
> +
> +	mddev->queue = disk->queue;
> +	blk_set_stacking_limits(&mddev->queue->limits);
>   	blk_queue_write_cache(mddev->queue, true, true);
>   	/* Allow extended partitions.  This makes the
>   	 * 'mdp' device redundant, but we can't really
> 
Wouldn't it make sense to introduce a helper 'blk_queue_from_disk()' or 
somesuch to avoid having to keep an explicit 'queue' pointer?


Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 15/26] nvdimm-blk: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  8:13 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-16-hch@lst.de>

On 5/21/21 7:51 AM, Christoph Hellwig wrote:
> Convert the nvdimm-blk driver to use the blk_alloc_disk and
> blk_cleanup_disk helpers to simplify gendisk and request_queue
> allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/nvdimm/blk.c | 26 ++++++--------------------
>   1 file changed, 6 insertions(+), 20 deletions(-)
> Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply

* Re: [PATCH 16/26] nvdimm-btt: convert to blk_alloc_disk/blk_cleanup_disk
From: Hannes Reinecke @ 2021-05-23  8:14 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Geert Uytterhoeven, Chris Zankel,
	Max Filippov, Philipp Reisner, Lars Ellenberg, Jim Paris,
	Philip Kelleher, Minchan Kim, Nitin Gupta, Matias Bjorling,
	Coly Li, Mike Snitzer, Song Liu, Maxim Levitsky, Alex Dubov,
	Ulf Hansson, Dan Williams, Vishal Verma, Dave Jiang,
	Heiko Carstens, Vasily Gorbik, Christian Borntraeger
  Cc: linux-bcache, linux-xtensa, linux-raid, nvdimm, linux-s390,
	linux-mmc, linux-m68k, linux-nvme, linux-block, dm-devel,
	linuxppc-dev, drbd-dev
In-Reply-To: <20210521055116.1053587-17-hch@lst.de>

On 5/21/21 7:51 AM, Christoph Hellwig wrote:
> Convert the nvdimm-btt driver to use the blk_alloc_disk and
> blk_cleanup_disk helpers to simplify gendisk and request_queue
> allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/nvdimm/btt.c | 24 +++++++-----------------
>   drivers/nvdimm/btt.h |  2 --
>   2 files changed, 7 insertions(+), 19 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox