Re: [PATCH][next] scsi: pm80xx: Fix potential infinite loop
On 07/04/2021 15:58, Colin King wrote:
> From: Colin Ian King
>
> The for-loop iterates with a u8 loop counter i and compares this
> with the loop upper limit of pm8001_ha->max_q_num which is a u32
> type. There is a potential infinite loop if pm8001_ha->max_q_num
> is larger than the u8 loop counter. Fix this by making the loop
> counter the same type as pm8001_ha->max_q_num.
Heh, coincidentally I've read your blog post on this issue today.
> Addresses-Coverity: ("Infinite loop")
> Fixes: 65df7d1986a1 ("scsi: pm80xx: Fix chip initialization failure")
AFAICS this still is in Martin's tree and not yet in Linus' tree.
Anyways, looks good.
Reviewed-by: Johannes Thumshirn
Re: [PATCH] scsi: lpfc: Remove unneeded return variable
On 04/02/2021 08:37, Yang Li wrote:
> This patch removes unneeded return variables, using only
> '1' instead.
> It fixes the following warning detected by coccinelle:
>
> Reported-by: Abaci Robot
> Signed-off-by: Yang Li
> ---
> drivers/scsi/lpfc/lpfc_sli.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> index 95caad7..31f97f6 100644
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -3376,7 +3376,6 @@ struct lpfc_hbq_init *lpfc_hbq_defs[] = {
> struct lpfc_iocbq *saveq)
> {
> struct lpfc_iocbq *cmdiocbp;
> - int rc = 1;
> unsigned long iflag;
>
> cmdiocbp = lpfc_sli_iocbq_lookup(phba, pring, saveq);
> @@ -3501,7 +3500,7 @@ struct lpfc_hbq_init *lpfc_hbq_defs[] = {
> }
> }
>
> - return rc;
> + return 1;
> }
>
> /**
>
I suppose this is in 'lpfc_sli_process_sol_iocb()'. It's return
value doesn't get handled either:
case LPFC_SOL_IOCB:
spin_unlock_irqrestore(>hbalock, iflag);
rc = lpfc_sli_process_sol_iocb(phba, pring, saveq);
spin_lock_irqsave(>hbalock, iflag);
break;
[...]
}
if (free_saveq) {
list_for_each_entry_safe(rspiocbp, next_iocb,
>list, list) {
list_del_init(>list);
__lpfc_sli_release_iocbq(phba, rspiocbp);
}
__lpfc_sli_release_iocbq(phba, saveq);
}
rspiocbp = NULL;
}
spin_unlock_irqrestore(>hbalock, iflag);
return rspiocbp;
}
Re: [PATCH] scsi: qla4xxx: Remove unneeded return variable
On 04/02/2021 08:20, Yang Li wrote:
> This patch removes unneeded return variables, using only
> '0' instead.
> It fixes the following warning detected by coccinelle:
> ./drivers/scsi/qla4xxx/ql4_os.c:3642:5-7: Unneeded variable: "rc".
> Return "0" on line 3741
>
> Reported-by: Abaci Robot
> Signed-off-by: Yang Li
> ---
> drivers/scsi/qla4xxx/ql4_os.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index a4b014e..ed92534 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -3639,7 +3639,6 @@ static int qla4xxx_copy_to_fwddb_param(struct
> iscsi_bus_flash_session *sess,
> struct dev_db_entry *fw_ddb_entry)
> {
> uint16_t options;
> - int rc = 0;
>
> options = le16_to_cpu(fw_ddb_entry->options);
> SET_BITVAL(conn->is_fw_assigned_ipv6, options, BIT_11);
> @@ -3738,7 +3737,7 @@ static int qla4xxx_copy_to_fwddb_param(struct
> iscsi_bus_flash_session *sess,
>
> COPY_ISID(fw_ddb_entry->isid, sess->isid);
>
> - return rc;
> + return 0;
> }
>
> static void qla4xxx_copy_to_sess_conn_params(struct iscsi_conn *conn,
>
None of the callers checks the return value anyways so you can do:
- static int qla4xxx_copy_to_fwddb_param(struct iscsi_bus_flash_session *sess,
+ static void qla4xxx_copy_to_fwddb_param(struct iscsi_bus_flash_session *sess,
Re: [PATCH] scsi: ipr: Remove unneeded return variable
On 04/02/2021 08:28, Yang Li wrote:
> This patch removes unneeded return variables, using only
> '0' instead.
> It fixes the following warning detected by coccinelle:
> ./drivers/scsi/ipr.c:9508:5-7: Unneeded variable: "rc". Return "0" on
> line 9524
>
> Reported-by: Abaci Robot
> Signed-off-by: Yang Li
> ---
> drivers/scsi/ipr.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index e451102..8eced7c 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -9505,7 +9505,6 @@ static pci_ers_result_t ipr_pci_error_detected(struct
> pci_dev *pdev,
> **/
> static int ipr_probe_ioa_part2(struct ipr_ioa_cfg *ioa_cfg)
> {
> - int rc = 0;
> unsigned long host_lock_flags = 0;
>
> ENTER;
> @@ -9521,7 +9520,7 @@ static int ipr_probe_ioa_part2(struct ipr_ioa_cfg
> *ioa_cfg)
> spin_unlock_irqrestore(ioa_cfg->host->host_lock, host_lock_flags);
>
> LEAVE;
> - return rc;
> + return 0;
> }
>
> /**
>
As it's always returning 0 this is dead code as well:
rc = ipr_probe_ioa_part2(ioa_cfg);
if (rc) {
__ipr_remove(pdev);
return rc;
}
I think:
- static int ipr_probe_ioa_part2(struct ipr_ioa_cfg *ioa_cfg)
+ static void ipr_probe_ioa_part2(struct ipr_ioa_cfg *ioa_cfg)
is the right thing to do if you really want to touch it.
Re: [PATCH] nvme: reject the ns when the block size is smaller than a sector
On 13/01/2021 17:07, Li Feng wrote:
> The nvme spec(1.4a, figure 248) says:
> "A value smaller than 9 (i.e., 512 bytes) is not supported."
>
> Signed-off-by: Li Feng
> ---
> drivers/nvme/host/core.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index f320273fc672..1f02e6e49a05 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2161,6 +2161,12 @@ static int nvme_update_ns_info(struct nvme_ns *ns,
> struct nvme_id_ns *id)
>
> blk_mq_freeze_queue(ns->disk->queue);
> ns->lba_shift = id->lbaf[lbaf].ds;
> + if (ns->lba_shift < 9) {
> + pr_warn("%s: bad lba_shift(%d)\n", ns->disk->disk_name,
> ns->lba_shift);
> + ret = -1;
> + goto out_unfreeze;
> + }
> +
> nvme_set_queue_limits(ns->ctrl, ns->queue);
>
> if (ns->head->ids.csi == NVME_CSI_ZNS) {
>
But this only catches a physical block size < 512 for NVMe, not any other block
device.
Please fix it for the general case in blk_queue_physical_block_size().
Thanks,
Johannes
Re: [PATCH] scsi: mpt3sas: Simplify _base_display_OEMs_branding
On 12/01/2021 18:38, Joe Perches wrote:
> static void
> _base_display_OEMs_branding(struct MPT3SAS_ADAPTER *ioc)
> {
> + const char *b = NULL; /* brand */
> + const char *v = NULL; /* vendor */
Any reason you didn't spell out brand and vendor as the variable names?
Re: [PATCH v2] blk: avoid divide-by-zero with zero granularity
On 12/01/2021 18:29, Feng Li wrote: > I use the nvme-tcp as the host, the target is spdk nvme-tcp target, > and set a wrong block size(i.g. bs=8), then the host prints this oops: I think the better fix here is to reject devices which report a block size small than a sector.
Re: [PATCH] Documentation: document dma device use for mcb
On 11/01/2021 21:27, Jonathan Corbet wrote: > Sorry, I've been distracted by holidays, merge window, network issues, and > generally watching the news in horror. The patch is applied now, thanks. Thanks Jon
Re: [PATCH] Documentation: document dma device use for mcb
On 18/12/2020 16:35, Johannes Thumshirn wrote: > Hannes reported a problem with setting up dma transfers on a mcb device. > The problem boiled down to the use of a wrong 'device' for the dma > functions. > > Document how to setup dma transfers for a IP core on a mcb carrier. Ping?
[PATCH] Documentation: document dma device use for mcb
Hannes reported a problem with setting up dma transfers on a mcb device. The problem boiled down to the use of a wrong 'device' for the dma functions. Document how to setup dma transfers for a IP core on a mcb carrier. Reported-by: Hannes Duerr Signed-off-by: Johannes Thumshirn --- Documentation/driver-api/men-chameleon-bus.rst | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/driver-api/men-chameleon-bus.rst b/Documentation/driver-api/men-chameleon-bus.rst index 1b1f048aa748..6f0b9ee47595 100644 --- a/Documentation/driver-api/men-chameleon-bus.rst +++ b/Documentation/driver-api/men-chameleon-bus.rst @@ -18,6 +18,7 @@ MEN Chameleon Bus 4.1 The driver structure 4.2 Probing and attaching 4.3 Initializing the driver + 4.4 Using DMA Introduction @@ -173,3 +174,14 @@ module at the MCB core:: The module_mcb_driver() macro can be used to reduce the above code:: module_mcb_driver(foo_driver); + +Using DMA +- + +To make use of the kernel's DMA-API's function, you will need to use the +carrier device's 'struct device'. Fortunately 'struct mcb_device' embeds a +pointer (->dma_dev) to the carrier's device for DMA purposes:: + +ret = dma_set_mask_and_coherent(>dma_dev, DMA_BIT_MASK(dma_bits)); +if (rc) +/* Handle errors */ -- 2.26.2
Re: [RFC PATCH v3 1/2] block: add simple copy support
On 11/12/2020 15:57, SelvaKumar S wrote:
[...]
> +int blk_copy_emulate(struct block_device *bdev, struct blk_copy_payload
> *payload,
> + gfp_t gfp_mask)
> +{
> + struct request_queue *q = bdev_get_queue(bdev);
> + struct bio *bio;
> + void *buf = NULL;
> + int i, nr_srcs, max_range_len, ret, cur_dest, cur_size;
> +
> + nr_srcs = payload->copy_range;
> + max_range_len = q->limits.max_copy_range_sectors << SECTOR_SHIFT;
> + cur_dest = payload->dest;
> + buf = kvmalloc(max_range_len, GFP_ATOMIC);
Why GFP_ATOMIC and not the passed in gfp_mask? Especially as this is a
kvmalloc()
which has the potential to grow quite big.
> +int __blkdev_issue_copy(struct block_device *bdev, sector_t dest,
> + sector_t nr_srcs, struct range_entry *rlist, gfp_t gfp_mask,
> + int flags, struct bio **biop)
> +{
[...]
> + total_size = struct_size(payload, range, nr_srcs);
> + payload = kmalloc(total_size, GFP_ATOMIC | __GFP_NOWARN);
Same here.
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 6b785181344f..a4a507d85e56 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -142,6 +142,47 @@ static int blk_ioctl_discard(struct block_device *bdev,
> fmode_t mode,
> GFP_KERNEL, flags);
> }
>
> +static int blk_ioctl_copy(struct block_device *bdev, fmode_t mode,
> + unsigned long arg, unsigned long flags)
> +{
[...]
> +
> + rlist = kmalloc_array(crange.nr_range, sizeof(*rlist),
> + GFP_ATOMIC | __GFP_NOWARN);
And here. I think this one can even be GFP_KERNEL.
Re: [RFC PATCH v2 0/2] add simple copy support
On 08/12/2020 14:13, Javier González wrote: > On 08.12.2020 12:37, Johannes Thumshirn wrote: >> On 08/12/2020 13:22, Javier González wrote: >>> Good idea. Are you thinking of a sysfs entry to select the backend? >> >> Not sure on this one, initially I thought of a sysfs file, but then >> how would you do it. One "global" sysfs entry is probably a bad idea. >> Having one per block device to select native vs emulation maybe? And >> a good way to benchmark. > > I was thinking a per block device to target the use case where a certain > implementation / workload is better one way or the other. Yes something along those lines. >> >> The other idea would be a benchmark loop on boot like the raid library >> does. >> >> Then on the other hand, there might be workloads that run faster with >> the emulation and some that run faster with the hardware acceleration. >> >> I think these points are the reason the last attempts got stuck. > > Yes. I believe that any benchmark we run would be biased in a certain > way. If we can move forward with a sysfs entry and default to legacy > path, we would not alter current behavior and enable NVMe copy offload > (for now) for those that want to use it. We can then build on top of it. > > Does this sound like a reasonable approach? > Yes this sounds like a reasonable approach to me.
Re: [RFC PATCH v2 0/2] add simple copy support
On 08/12/2020 13:22, Javier González wrote: > Good idea. Are you thinking of a sysfs entry to select the backend? Not sure on this one, initially I thought of a sysfs file, but then how would you do it. One "global" sysfs entry is probably a bad idea. Having one per block device to select native vs emulation maybe? And a good way to benchmark. The other idea would be a benchmark loop on boot like the raid library does. Then on the other hand, there might be workloads that run faster with the emulation and some that run faster with the hardware acceleration. I think these points are the reason the last attempts got stuck.
Re: [RFC PATCH v2 0/2] add simple copy support
On 07/12/2020 20:27, Javier González wrote: > Good point. We can share some performance data on how Simple Copy scales > in terms of bw / latency and the CPU usage. Do you have anything else in > mind? > With an emulation in the kernel, we could make the usd "backend" implementation configurable. So if the emulation is faster, users can select the emulation, if the device is faster then the device. Kind of what the crypto and raid code do as well. I'm really interested in this work, as BTRFS relocation/balance will have potential benefits, but we need to get it right.
Re: [RFC PATCH 1/2] block: add simple copy support
On 01/12/2020 08:14, SelvaKumar S wrote:
> +static inline int bio_check_copy_eod(struct bio *bio, sector_t start,
> + sector_t nr_sectors, sector_t maxsector)
> +{
> + if (nr_sectors && maxsector && (nr_sectors > maxsector ||
> + start > maxsector - nr_sectors)) {
Nit: I don't like the line break here, maybe:
if (nr_sectors && maxsector &&
(nr_sectors > maxsector || start > maxsector - nr_sectors)) {
> + handle_bad_sector(bio, maxsector);
> + return -EIO;
> + }
> + return 0;
> +}
> +
> /*
> * Check whether this bio extends beyond the end of the device or partition.
> * This may well happen - the kernel calls bread() without checking the size
> of
> @@ -737,6 +748,75 @@ static inline int bio_check_eod(struct bio *bio,
> sector_t maxsector)
> return 0;
> }
>
> +/*
> + * Check for copy limits and remap source ranges if needed.
> + */
> +static inline int blk_check_copy(struct bio *bio)
That function is a bit big to be marked as inline, isn't it?
> +{
> + struct hd_struct *p = NULL;
> + struct request_queue *q = bio->bi_disk->queue;
> + struct blk_copy_payload *payload;
> + unsigned short nr_range;
> + int ret = -EIO;
> + int i, copy_len = 0;
> +
> + rcu_read_lock();
> +
> + if (bio->bi_partno) {
> + p = __disk_get_part(bio->bi_disk, bio->bi_partno);
> + if (unlikely(!p))
> + goto out;
> + if (unlikely(bio_check_ro(bio, p)))
> + goto out;
> + } else {
> + if (unlikely(bio_check_ro(bio, >bi_disk->part0)))
> + goto out;
> + }
> +
> + payload = bio_data(bio);
> + nr_range = payload->copy_range;
> +
> + /* cannot handle copy crossing nr_ranges limit */
> + if (payload->copy_range > q->limits.max_copy_nr_ranges)
> + goto out;
> +
> + for (i = 0; i < nr_range; i++) {
> + copy_len += payload->range[i].len;
> + if (p) {
> + if (bio_check_copy_eod(bio, payload->range[i].src,
> + payload->range[i].len,
> part_nr_sects_read(p)))
> + goto out;
> + payload->range[i].src += p->start_sect;
> + } else {
> + if (unlikely(bio_check_copy_eod(bio,
> payload->range[i].src,
> + payload->range[i].len,
> + get_capacity(bio->bi_disk
> + goto out;
> + }
> + }
> +
> + /* cannot handle copy more than copy limits */
> + if (copy_len > q->limits.max_copy_sectors)
> + goto out;
> +
> + if (p) {
> + if (unlikely(bio_check_copy_eod(bio, bio->bi_iter.bi_sector,
> copy_len,
> + part_nr_sects_read(p
> + goto out;
> + } else {
> + if (unlikely(bio_check_copy_eod(bio, bio->bi_iter.bi_sector,
> copy_len,
> + get_capacity(bio->bi_disk
> + goto out;
> +
> + }
> +
All these if (p) {} else {} branches make this function a bit hard to follow.
> + if (p)
> + bio->bi_partno = 0;
> + ret = 0;
> +out:
> + rcu_read_unlock();
> + return ret;
> +}
> /*
> * Remap block n of partition p to block n+start(p) of the disk.
> */
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index e90614fd8d6a..db4947f7014d 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -150,6 +150,122 @@ int blkdev_issue_discard(struct block_device *bdev,
> sector_t sector,
> }
> EXPORT_SYMBOL(blkdev_issue_discard);
>
> +int __blkdev_issue_copy(struct block_device *bdev, sector_t dest,
> + sector_t nr_srcs, struct range_entry *rlist, gfp_t gfp_mask,
> + int flags, struct bio **biop)
> +{
> + struct request_queue *q = bdev_get_queue(bdev);
> + struct bio *bio;
> + struct blk_copy_payload *payload;
> + unsigned int op;
I don't think op is needed.
> + sector_t bs_mask;
> + sector_t src_sects, len = 0, total_len = 0;
> + int i, ret, total_size;
> +
> + if (!q)
> + return -ENXIO;
> +
> + if (!nr_srcs)
> + return -EINVAL;
> +
> + if (bdev_read_only(bdev))
> + return -EPERM;
> +
> + if (!blk_queue_copy(q))
> + return -EOPNOTSUPP;
> + op = REQ_OP_COPY;
> +
> + bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
> + if (dest & bs_mask)
> + return -EINVAL;
> +
> + payload = kmalloc(sizeof(struct blk_copy_payload) +
> + nr_srcs * sizeof(struct range_entry),
> + GFP_ATOMIC | __GFP_NOWARN);
Please check if the use of struct_size() is possible. Probably even assign
total_size here so you don't need to do the size
Re: [PATCH RESEND] MAINTAINERS: Update maintainer entries for MEN HW
On 03/06/2020 09:10, AGeissler wrote: > [resend; because of missing cc] > > Remove Andreas Werner as Maintainer of the F21 BMC driver, as he is no > longer with the company and add Andreas Geissler as additional > Maintainer for all MEN Hardware. > > Acked-by: Johannes Thumshirn > Acked-by: Andreas Werner > Signed-off-by: AGeissler > --- > MAINTAINERS | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 68cd1b966b45..b3b45d2e9c8a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11044,19 +11044,21 @@ F: include/uapi/mtd/ > > MEN A21 WATCHDOG DRIVER > M: Johannes Thumshirn > +M: Andreas Geissler > L: linux-watch...@vger.kernel.org > S: Maintained > F: drivers/watchdog/mena21_wdt.c > > MEN CHAMELEON BUS (mcb) > M: Johannes Thumshirn > +M: Andreas Geissler > S: Maintained > F: Documentation/driver-api/men-chameleon-bus.rst > F: drivers/mcb/ > F: include/linux/mcb.h > > MEN F21BMC (Board Management Controller) > -M: Andreas Werner > +M: Andreas Geissler > S: Supported > F: Documentation/hwmon/menf21bmc.rst > F: drivers/hwmon/menf21bmc_hwmon.c > @@ -11066,10 +11068,18 @@ F: drivers/watchdog/menf21bmc_wdt.c > > MEN Z069 WATCHDOG DRIVER > M: Johannes Thumshirn > +M: Andreas Geissler > L: linux-watch...@vger.kernel.org > S: Maintained > F: drivers/watchdog/menz69_wdt.c > > +MEN Z135 UART DRIVER > +M: Johannes Thumshirn > +M: Andreas Geissler > +L: linux-ser...@vger.kernel.org > +S: Maintained > +F: drivers/tty/serial/men_z135_uart.c > + > MESON AO CEC DRIVER FOR AMLOGIC SOCS > M: Neil Armstrong > L: linux-me...@vger.kernel.org > > base-commit: d6f9469a03d832dcd17041ed67774ffb5f3e73b3 > Looks like this never got applied. Greg can you pick it up?
Re: linux-next: Fixes tag needs some work in the btrfs-fixes tree
On 26/10/2020 16:01, Stephen Rothwell wrote:
> Hi all,
>
> In commit
>
> ae1a53ee21a5 ("btrfs: don't fallback to buffered read if we don't need to")
>
> Fixes tag
>
> Fixes: b5ff9f1a96e8f ("btrfs: switch to iomap for direct IO")
>
> has these problem(s):
>
> - Target SHA1 does not exist
>
> Maybe you meant
>
> Fixes: f85781fb505e ("btrfs: switch to iomap for direct IO")
>
Hmm b5ff9f1a96e8f exists in my tree. Probably something stale.
Re: [PATCH 1/2] Block layer filter - second version
On 21/10/2020 11:04, Sergei Shtepa wrote: > + help > + Enabling this lets third-party kernel modules intercept > + bio requests for any block device. This allows them to implement The "third-party kernel modules" part sounds a bit worrisome to me. Especially as this functionality is based on EXPORT_SYMBOL()s without the GPL suffix. I read it as a "allow a proprietary module to mess with bios", which is a big no-no to me. Not providing any sort of changelog doesn't help much either. Thanks, Johannes
Re: [PATCH] blk: use REQ_OP_WRITE instead of hard code
On 19/10/2020 16:30, 苏辉 wrote: > Yeah, you are right, thanks for your explanationMaybe we should > define a MASK to do this? Why? I personally find a '& 1' way more understandable than a REQ_OP_IS_WRITE_MASK or sth like that. The former I can just read, for the latter I would need to look up the definition to be able to understand the code.
Re: [PATCH] blk: use REQ_OP_WRITE instead of hard code
On 19/10/2020 16:06, Hui Su wrote:
> use REQ_OP_WRITE instead of hard code in
> op_is_write().
>
> Signed-off-by: Hui Su
> ---
> include/linux/blk_types.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index 7d7c13238fdb..7b9b02378c24 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -440,7 +440,7 @@ static inline void bio_set_op_attrs(struct bio *bio,
> unsigned op,
>
> static inline bool op_is_write(unsigned int op)
> {
> - return (op & 1);
> + return (op & REQ_OP_WRITE);
op_is_write() returns true for every req_op that writes to a device (they all
have
the lowest bit set), while REQ_OP_WRITE means a WRITE. You'll change the
semantics
with this patch.
Re: 回复: 回复: [PATCH] btrfs: Fix missing close devices
On 23/09/2020 08:03, Zhang, Qiang wrote: > Hello Johannes Thumshirn > > the crash happend in "snprintf(s->s_id, sizeof(s->s_id), "%pg", bdev)" in > btrfs_mount_root func, the "bdev" may be destroyed in btrfs_close_devices. > I think add btrfs_close_devices func before deactivate_locked_super is > reasonable. > I'm not sure if that's another problem . > What's your point of view ？ > I think this is generally unneeded. Think of this call chain: deactivate_locked_super() `-> fs->kill_sb() `-> btrfs_kill_super() `-> kill_anon_super() `-> generic_shutdown_super() `-> sop->put_super() `-> btrfs_put_super() `-> close_ctree() `-> btrfs_close_devices()
Re: [PATCH v3] null_blk: add support for max open/active zone limit for zoned devices
On 17/09/2020 09:57, Niklas Cassel wrote: > On Mon, Sep 07, 2020 at 08:18:26AM +, Niklas Cassel wrote: >> On Fri, Aug 28, 2020 at 12:54:00PM +0200, Niklas Cassel wrote: >>> Add support for user space to set a max open zone and a max active zone >>> limit via configfs. By default, the default values are 0 == no limit. >>> >>> Call the block layer API functions used for exposing the configured >>> limits to sysfs. >>> >>> Add accounting in null_blk_zoned so that these new limits are respected. >>> Performing an operation that would exceed these limits results in a >>> standard I/O error. >>> >>> A max open zone limit exists in the ZBC standard. >>> While null_blk_zoned is used to test the Zoned Block Device model in >>> Linux, when it comes to differences between ZBC and ZNS, null_blk_zoned >>> mostly follows ZBC. >>> >>> Therefore, implement the manage open zone resources function from ZBC, >>> but additionally add support for max active zones. >>> This enables user space not only to test against a device with an open >>> zone limit, but also to test against a device with an active zone limit. >>> >>> Signed-off-by: Niklas Cassel >>> Reviewed-by: Damien Le Moal >>> --- >>> Changes since v2: >>> -Picked up Damien's Reviewed-by tag. >>> -Fixed a typo in the commit message. >>> -Renamed null_manage_zone_resources() to null_has_zone_resources(). >>> >>> drivers/block/null_blk.h | 5 + >>> drivers/block/null_blk_main.c | 16 +- >>> drivers/block/null_blk_zoned.c | 319 +++-- >>> 3 files changed, 282 insertions(+), 58 deletions(-) >> >> Hello Jens, >> >> A gentle ping on this. >> >> As far as I can tell, there are no outstanding review comments. > > > Hello Jens, > > Pinging you from another address, in case my corporate email is getting > stuck in your spam filter. > > Kind regards, > Niklas > Jens, Any chance we can get this queued up for 5.10? This is really helpful for e.g. the zonefs test suite or xfstests when btrfs HMZONED support lands. Thanks, Johannes
Re: KASAN: stack-out-of-bounds Write in read_extent_buffer
On 21/09/2020 16:32, syzbot wrote: > Hello, > > syzbot found the following issue on: > > HEAD commit:860461e4 Add linux-next specific files for 20200917 > git tree: linux-next > console output: https://syzkaller.appspot.com/x/log.txt?x=141fc5d990 > kernel config: https://syzkaller.appspot.com/x/.config?x=f2392812d6c63d5c > dashboard link: https://syzkaller.appspot.com/bug?extid=1d393803acac53c985a0 > compiler: gcc (GCC) 10.1.0-syz 20200507 > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16778b0790 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1216f8ad90 > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+1d393803acac53c98...@syzkaller.appspotmail.com > > == > BUG: KASAN: stack-out-of-bounds in memcpy include/linux/string.h:399 [inline] > BUG: KASAN: stack-out-of-bounds in read_extent_buffer+0x114/0x150 > fs/btrfs/extent_io.c:5674 > Write of size 8 at addr c9dd79f0 by task kworker/u4:1/21 > > CPU: 1 PID: 21 Comm: kworker/u4:1 Not tainted > 5.9.0-rc5-next-20200917-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Workqueue: btrfs-endio-meta btrfs_work_helper > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0x198/0x1fb lib/dump_stack.c:118 > print_address_description.constprop.0.cold+0x5/0x497 mm/kasan/report.c:385 > __kasan_report mm/kasan/report.c:545 [inline] > kasan_report.cold+0x1f/0x37 mm/kasan/report.c:562 > check_memory_region_inline mm/kasan/generic.c:186 [inline] > check_memory_region+0x13d/0x180 mm/kasan/generic.c:192 > memcpy+0x39/0x60 mm/kasan/common.c:106 > memcpy include/linux/string.h:399 [inline] > read_extent_buffer+0x114/0x150 fs/btrfs/extent_io.c:5674 > btree_readpage_end_io_hook+0x7de/0x950 fs/btrfs/disk-io.c:641 > end_bio_extent_readpage+0x4de/0x10c0 fs/btrfs/extent_io.c:2854 > bio_endio+0x3d3/0x7a0 block/bio.c:1449 > end_workqueue_fn+0x114/0x170 fs/btrfs/disk-io.c:1696 > btrfs_work_helper+0x20a/0xd20 fs/btrfs/async-thread.c:318 > process_one_work+0x933/0x15a0 kernel/workqueue.c:2269 > worker_thread+0x64c/0x1120 kernel/workqueue.c:2415 > kthread+0x3af/0x4a0 kernel/kthread.c:292 > ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:296 > > > addr c9dd79f0 is located in stack of task kworker/u4:1/21 at offset > 48 in frame: > btree_readpage_end_io_hook+0x0/0x950 fs/btrfs/disk-io.c:201 > > this frame has 4 objects: > [48, 52) 'val' > [64, 80) 'fsid' > [96, 128) 'result' > [160, 192) 'found' > > Memory state around the buggy address: > c9dd7880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > c9dd7900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> c9dd7980: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 f1 f1 04 f2 > ^ > c9dd7a00: 00 00 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00 00 00 > c9dd7a80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 > == > > > --- > This report is generated by a bot. It may contain errors. > See https://goo.gl/tpsmEJ for more information about syzbot. > syzbot engineers can be reached at syzkal...@googlegroups.com. > > syzbot will keep track of this issue. See: > https://goo.gl/tpsmEJ#status for how to communicate with syzbot. > syzbot can test patches for this issue, for details see: > https://goo.gl/tpsmEJ#testing-patches > #syz test: git://github.com/kdave/btrfs-devel.git misc-5.9
Re: [PATCH] btrfs: Fix missing close devices
On 21/09/2020 13:00, qiang.zh...@windriver.com wrote:
> From: Zqiang
>
> When the btrfs fill super error, we should first close devices and
> then call deactivate_locked_super func to free fs_info.
>
> Signed-off-by: Zqiang
> ---
> fs/btrfs/super.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 8840a4fa81eb..3bfd54e8f388 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1675,6 +1675,7 @@ static struct dentry *btrfs_mount_root(struct
> file_system_type *fs_type,
> error = security_sb_set_mnt_opts(s, new_sec_opts, 0, NULL);
> security_free_mnt_opts(_sec_opts);
> if (error) {
> + btrfs_close_devices(fs_devices);
> deactivate_locked_super(s);
> return ERR_PTR(error);
> }
>
Hmm you didn't change anything, so my report in [1] still exists.
[1]
https://lore.kernel.org/r/sn4pr0401mb359820738ac6479f9f47fee59b...@sn4pr0401mb3598.namprd04.prod.outlook.com
Re: 回复: [PATCH] btrfs: Fix missing close devices
On 21/09/2020 11:14, Zhang, Qiang wrote:
>
>
> ____
> 发件人: Johannes Thumshirn
> 发送时间: 2020年9月21日 16:52
> 收件人: Zhang, Qiang; c...@fb.com; jo...@toxicpanda.com; dste...@suse.com
> 抄送: linux-bt...@vger.kernel.org; linux-kernel@vger.kernel.org
> 主题: Re: [PATCH] btrfs: Fix missing close devices
>
> On 21/09/2020 10:27, qiang.zh...@windriver.com wrote:
>> From: Zqiang
>>
>> When the btrfs fill super error, we should first close devices and
>> then call deactivate_locked_super func to free fs_info.
>>
>> Signed-off-by: Zqiang
>> ---
>> fs/btrfs/super.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 8840a4fa81eb..3bfd54e8f388 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -1675,6 +1675,7 @@ static struct dentry *btrfs_mount_root(struct
>> file_system_type *fs_type,
>> error = security_sb_set_mnt_opts(s, new_sec_opts, 0, NULL);
>> security_free_mnt_opts(_sec_opts);
>> if (error) {
>> + btrfs_close_devices(fs_devices);
>> deactivate_locked_super(s);
>> return ERR_PTR(error);
>> }
>>
>
>> I think this is the fix for the syzkaller issue:
>> Reported-by: syzbot+582e66e5edf36a22c...@syzkaller.appspotmail.com
>
> Please try this patch.
>
Nope, with this patch I get the following Null-ptr-deref:
[ 39.065209]
==
[ 39.066318] BUG: KASAN: null-ptr-deref in bdev_name.constprop.0+0xd4/0x240
[ 39.067307] Read of size 4 at addr 03ac by task syz-repro/273
[ 39.068289]
[ 39.069602]
==
[ 39.070837] BUG: kernel NULL pointer dereference, address: 03ac
[ 39.071837] #PF: supervisor read access in kernel mode
[ 39.072580] #PF: error_code(0x) - not-present page
[ 39.073318] PGD 8001cd3b1067 P4D 8001cd3b1067 PUD 1c6de7067 PMD 0
[ 39.074306] Oops: [#1] SMP KASAN PTI
[ 39.074887] CPU: 0 PID: 273 Comm: syz-repro Tainted: GB
5.9.0-rc5+ #772
[ 39.076031] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.13.0-0-gf21b5a4-rebuilt.opensuse.org 04/01/2014
[ 39.077638] RIP: 0010:bdev_name.constprop.0+0xd4/0x240
[ 39.078387] Code: ca 4c 89 4c 24 08 e8 0b e9 ff ff 48 89 df 49 89 c6 e8 40
42 c6 ff 49 8b ac 24 e0 00 00 00 48 8d bd ac 03 00 00 e8 2c 41 c6 ff <8b> 85 ac
03 00 00 4c 8b 4c 24 08 85 c0 0f 84 fe 00 00 00 4c 89 cf
[ 39.080991] RSP: 0018:8881f1a97878 EFLAGS: 00010286
[ 39.081728] RAX: 0001 RBX: 8881c9fb80e0 RCX: dc00
[ 39.082725] RDX: 0007 RSI: 0004 RDI: 81acd784
[ 39.083717] RBP: R08: R09:
[ 39.084722] R10: fbfff0539591 R11: 0001 R12: 8881c9fb8000
[ 39.085711] R13: 8881ef6e2698 R14: 8881ef6e2680 R15:
[ 39.086704] FS: 7f5d36eb9540() GS:8881f760()
knlGS:
[ 39.087827] CS: 0010 DS: ES: CR0: 80050033
[ 39.088623] CR2: 03ac CR3: 0001ef552000 CR4: 06b0
[ 39.089607] DR0: DR1: DR2:
[ 39.090603] DR3: DR6: fffe0ff0 DR7: 0400
[ 39.091583] Call Trace:
[ 39.091943] ? mac_address_string+0x380/0x380
[ 39.092559] ? mark_held_locks+0x65/0x90
[ 39.093116] pointer+0x21c/0x650
[ 39.093578] ? format_decode+0x1cf/0x4e0
[ 39.094139] ? resource_string.isra.0+0xc10/0xc10
[ 39.094809] vsnprintf+0x2e0/0x820
[ 39.095292] ? pointer+0x650/0x650
[ 39.095785] snprintf+0x88/0xa0
[ 39.096234] ? vsprintf+0x10/0x10
[ 39.096708] ? rcu_read_lock_sched_held+0x3a/0x70
[ 39.097378] ? sget+0x200/0x240
[ 39.097908] ? btrfs_kill_super+0x30/0x30 [btrfs]
[ 39.098644] btrfs_mount_root+0x442/0x5d0 [btrfs]
[ 39.099377] ? parse_rescue_options+0x150/0x150 [btrfs]
[ 39.100103] ? rcu_read_lock_sched_held+0x3a/0x70
[ 39.100759] ? vfs_parse_fs_string+0xbc/0xf0
[ 39.101355] ? kfree+0x1e0/0x31
Re: KASAN: use-after-free Read in btrfs_scan_one_device
On 21/09/2020 07:38, syzbot wrote: > syzbot has found a reproducer for the following issue on: > > HEAD commit:325d0eab Merge branch 'akpm' (patches from Andrew) > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=1512df5390 > kernel config: https://syzkaller.appspot.com/x/.config?x=6a8a2ae52ed737db > dashboard link: https://syzkaller.appspot.com/bug?extid=582e66e5edf36a22c7b0 > compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ > c2443155a0fb245c8f17f2c1c72b6ea391e86e81) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12366f8b90 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14e6929b90 > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+582e66e5edf36a22c...@syzkaller.appspotmail.com > > == > BUG: KASAN: use-after-free in btrfs_printk+0x3eb/0x435 fs/btrfs/super.c:245 > Read of size 8 at addr 8880878e06a8 by task syz-executor225/7068 > > CPU: 1 PID: 7068 Comm: syz-executor225 Not tainted 5.9.0-rc5-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0x1d6/0x29e lib/dump_stack.c:118 > print_address_description+0x66/0x620 mm/kasan/report.c:383 > __kasan_report mm/kasan/report.c:513 [inline] > kasan_report+0x132/0x1d0 mm/kasan/report.c:530 > btrfs_printk+0x3eb/0x435 fs/btrfs/super.c:245 > device_list_add+0x1a88/0x1d60 fs/btrfs/volumes.c:943 > btrfs_scan_one_device+0x196/0x490 fs/btrfs/volumes.c:1359 > btrfs_mount_root+0x48f/0xb60 fs/btrfs/super.c:1634 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > fc_mount fs/namespace.c:978 [inline] > vfs_kern_mount+0xc9/0x160 fs/namespace.c:1008 > btrfs_mount+0x33c/0xae0 fs/btrfs/super.c:1732 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > do_new_mount fs/namespace.c:2875 [inline] > path_mount+0x179d/0x29e0 fs/namespace.c:3192 > do_mount fs/namespace.c:3205 [inline] > __do_sys_mount fs/namespace.c:3413 [inline] > __se_sys_mount+0x126/0x180 fs/namespace.c:3390 > do_syscall_64+0x31/0x70 arch/x86/entry/common.c:46 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > RIP: 0033:0x44840a > Code: b8 08 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 cd a2 fb ff c3 66 2e 0f 1f > 84 00 00 00 00 00 66 90 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 0f > 83 aa a2 fb ff c3 66 0f 1f 84 00 00 00 00 00 > RSP: 002b:7ffedfffd608 EFLAGS: 0293 ORIG_RAX: 00a5 > RAX: ffda RBX: 7ffedfffd670 RCX: 0044840a > RDX: 2000 RSI: 2100 RDI: 7ffedfffd630 > RBP: 7ffedfffd630 R08: 7ffedfffd670 R09: > R10: R11: 0293 R12: 001a > R13: 0004 R14: 0003 R15: 0003 > > Allocated by task 6945: > kasan_save_stack mm/kasan/common.c:48 [inline] > kasan_set_track mm/kasan/common.c:56 [inline] > __kasan_kmalloc+0x100/0x130 mm/kasan/common.c:461 > kmalloc_node include/linux/slab.h:577 [inline] > kvmalloc_node+0x81/0x110 mm/util.c:574 > kvmalloc include/linux/mm.h:757 [inline] > kvzalloc include/linux/mm.h:765 [inline] > btrfs_mount_root+0xd0/0xb60 fs/btrfs/super.c:1613 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > fc_mount fs/namespace.c:978 [inline] > vfs_kern_mount+0xc9/0x160 fs/namespace.c:1008 > btrfs_mount+0x33c/0xae0 fs/btrfs/super.c:1732 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > do_new_mount fs/namespace.c:2875 [inline] > path_mount+0x179d/0x29e0 fs/namespace.c:3192 > do_mount fs/namespace.c:3205 [inline] > __do_sys_mount fs/namespace.c:3413 [inline] > __se_sys_mount+0x126/0x180 fs/namespace.c:3390 > do_syscall_64+0x31/0x70 arch/x86/entry/common.c:46 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > Freed by task 6945: > kasan_save_stack mm/kasan/common.c:48 [inline] > kasan_set_track+0x3d/0x70 mm/kasan/common.c:56 > kasan_set_free_info+0x17/0x30 mm/kasan/generic.c:355 > __kasan_slab_free+0xdd/0x110 mm/kasan/common.c:422 > __cache_free mm/slab.c:3418 [inline] > kfree+0x113/0x200 mm/slab.c:3756 > deactivate_locked_super+0xa7/0xf0 fs/super.c:335 > btrfs_mount_root+0x72b/0xb60 fs/btrfs/super.c:1678 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > fc_mount fs/namespace.c:978 [inline] > vfs_kern_mount+0xc9/0x160 fs/namespace.c:1008 > btrfs_mount+0x33c/0xae0 fs/btrfs/super.c:1732 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > do_new_mount fs/namespace.c:2875 [inline] > path_mount+0x179d/0x29e0 fs/namespace.c:3192 > do_mount fs/namespace.c:3205 [inline] > __do_sys_mount
Re: [PATCH] btrfs: Fix missing close devices
On 21/09/2020 10:27, qiang.zh...@windriver.com wrote:
> From: Zqiang
>
> When the btrfs fill super error, we should first close devices and
> then call deactivate_locked_super func to free fs_info.
>
> Signed-off-by: Zqiang
> ---
> fs/btrfs/super.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 8840a4fa81eb..3bfd54e8f388 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1675,6 +1675,7 @@ static struct dentry *btrfs_mount_root(struct
> file_system_type *fs_type,
> error = security_sb_set_mnt_opts(s, new_sec_opts, 0, NULL);
> security_free_mnt_opts(_sec_opts);
> if (error) {
> + btrfs_close_devices(fs_devices);
> deactivate_locked_super(s);
> return ERR_PTR(error);
> }
>
I think this is the fix for the syzkaller issue:
Reported-by: syzbot+582e66e5edf36a22c...@syzkaller.appspotmail.com
Re: KASAN: use-after-free Read in btrfs_scan_one_device
On 21/09/2020 07:38, syzbot wrote: > syzbot has found a reproducer for the following issue on: > > HEAD commit:325d0eab Merge branch 'akpm' (patches from Andrew) > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=1512df5390 > kernel config: https://syzkaller.appspot.com/x/.config?x=6a8a2ae52ed737db > dashboard link: https://syzkaller.appspot.com/bug?extid=582e66e5edf36a22c7b0 > compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ > c2443155a0fb245c8f17f2c1c72b6ea391e86e81) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12366f8b90 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14e6929b90 > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+582e66e5edf36a22c...@syzkaller.appspotmail.com > > == > BUG: KASAN: use-after-free in btrfs_printk+0x3eb/0x435 fs/btrfs/super.c:245 > Read of size 8 at addr 8880878e06a8 by task syz-executor225/7068 > > CPU: 1 PID: 7068 Comm: syz-executor225 Not tainted 5.9.0-rc5-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0x1d6/0x29e lib/dump_stack.c:118 > print_address_description+0x66/0x620 mm/kasan/report.c:383 > __kasan_report mm/kasan/report.c:513 [inline] > kasan_report+0x132/0x1d0 mm/kasan/report.c:530 > btrfs_printk+0x3eb/0x435 fs/btrfs/super.c:245 > device_list_add+0x1a88/0x1d60 fs/btrfs/volumes.c:943 > btrfs_scan_one_device+0x196/0x490 fs/btrfs/volumes.c:1359 > btrfs_mount_root+0x48f/0xb60 fs/btrfs/super.c:1634 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > fc_mount fs/namespace.c:978 [inline] > vfs_kern_mount+0xc9/0x160 fs/namespace.c:1008 > btrfs_mount+0x33c/0xae0 fs/btrfs/super.c:1732 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > do_new_mount fs/namespace.c:2875 [inline] > path_mount+0x179d/0x29e0 fs/namespace.c:3192 > do_mount fs/namespace.c:3205 [inline] > __do_sys_mount fs/namespace.c:3413 [inline] > __se_sys_mount+0x126/0x180 fs/namespace.c:3390 > do_syscall_64+0x31/0x70 arch/x86/entry/common.c:46 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > RIP: 0033:0x44840a > Code: b8 08 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 cd a2 fb ff c3 66 2e 0f 1f > 84 00 00 00 00 00 66 90 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 0f > 83 aa a2 fb ff c3 66 0f 1f 84 00 00 00 00 00 > RSP: 002b:7ffedfffd608 EFLAGS: 0293 ORIG_RAX: 00a5 > RAX: ffda RBX: 7ffedfffd670 RCX: 0044840a > RDX: 2000 RSI: 2100 RDI: 7ffedfffd630 > RBP: 7ffedfffd630 R08: 7ffedfffd670 R09: > R10: R11: 0293 R12: 001a > R13: 0004 R14: 0003 R15: 0003 > > Allocated by task 6945: > kasan_save_stack mm/kasan/common.c:48 [inline] > kasan_set_track mm/kasan/common.c:56 [inline] > __kasan_kmalloc+0x100/0x130 mm/kasan/common.c:461 > kmalloc_node include/linux/slab.h:577 [inline] > kvmalloc_node+0x81/0x110 mm/util.c:574 > kvmalloc include/linux/mm.h:757 [inline] > kvzalloc include/linux/mm.h:765 [inline] > btrfs_mount_root+0xd0/0xb60 fs/btrfs/super.c:1613 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > fc_mount fs/namespace.c:978 [inline] > vfs_kern_mount+0xc9/0x160 fs/namespace.c:1008 > btrfs_mount+0x33c/0xae0 fs/btrfs/super.c:1732 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > do_new_mount fs/namespace.c:2875 [inline] > path_mount+0x179d/0x29e0 fs/namespace.c:3192 > do_mount fs/namespace.c:3205 [inline] > __do_sys_mount fs/namespace.c:3413 [inline] > __se_sys_mount+0x126/0x180 fs/namespace.c:3390 > do_syscall_64+0x31/0x70 arch/x86/entry/common.c:46 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > Freed by task 6945: > kasan_save_stack mm/kasan/common.c:48 [inline] > kasan_set_track+0x3d/0x70 mm/kasan/common.c:56 > kasan_set_free_info+0x17/0x30 mm/kasan/generic.c:355 > __kasan_slab_free+0xdd/0x110 mm/kasan/common.c:422 > __cache_free mm/slab.c:3418 [inline] > kfree+0x113/0x200 mm/slab.c:3756 > deactivate_locked_super+0xa7/0xf0 fs/super.c:335 > btrfs_mount_root+0x72b/0xb60 fs/btrfs/super.c:1678 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > fc_mount fs/namespace.c:978 [inline] > vfs_kern_mount+0xc9/0x160 fs/namespace.c:1008 > btrfs_mount+0x33c/0xae0 fs/btrfs/super.c:1732 > legacy_get_tree+0xea/0x180 fs/fs_context.c:592 > vfs_get_tree+0x88/0x270 fs/super.c:1547 > do_new_mount fs/namespace.c:2875 [inline] > path_mount+0x179d/0x29e0 fs/namespace.c:3192 > do_mount fs/namespace.c:3205 [inline] > __do_sys_mount
Re: WARNING in close_fs_devices (2)
On 21/09/2020 09:58, Anand Jain wrote: > On 21/9/20 6:42 am, syzbot wrote: >> syzbot has found a reproducer for the following issue on: >> >> HEAD commit:b652d2a5 Add linux-next specific files for 20200918 >> git tree: linux-next >> console output: https://syzkaller.appspot.com/x/log.txt?x=17e84b0790 >> kernel config: https://syzkaller.appspot.com/x/.config?x=3cf0782933432b43 >> dashboard link: https://syzkaller.appspot.com/bug?extid=4cfe71a4da060be47502 >> compiler: gcc (GCC) 10.1.0-syz 20200507 >> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=112425d990 >> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1486929b90 >> >> IMPORTANT: if you fix the issue, please add the following tag to the commit: >> Reported-by: syzbot+4cfe71a4da060be47...@syzkaller.appspotmail.com >> >> [ cut here ] >> WARNING: CPU: 0 PID: 6972 at fs/btrfs/volumes.c:1172 >> close_fs_devices+0x715/0x930 fs/btrfs/volumes.c:1172 >> Modules linked in: >> CPU: 1 PID: 6972 Comm: syz-executor044 Not tainted >> 5.9.0-rc5-next-20200918-syzkaller #0 >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS >> Google 01/01/2011 >> RIP: 0010:close_fs_devices+0x715/0x930 fs/btrfs/volumes.c:1172 >> Code: e8 00 b8 4c fe 85 db 0f 85 65 f9 ff ff e8 93 bb 4c fe 0f 0b e9 59 f9 >> ff ff e8 87 bb 4c fe 0f 0b e9 c0 fe ff ff e8 7b bb 4c fe <0f> 0b e9 f9 fe ff >> ff 48 c7 c7 fc a1 8f 8b e8 e8 0b 8e fe e9 19 f9 >> RSP: 0018:c900061b7758 EFLAGS: 00010293 >> RAX: RBX: RCX: 83285c2c >> RDX: 8880a6bbe4c0 RSI: 83285d35 RDI: 0007 >> RBP: dc00 R08: R09: 8880a2be1133 >> R10: R11: R12: 8880a2be1130 >> R13: 8880a2be11ec R14: 888093ab0508 R15: 8880a2be1050 >> FS: 0208a880() GS:8880ae50() knlGS: >> CS: 0010 DS: ES: CR0: 80050033 >> CR2: 004babec CR3: a7bc7000 CR4: 001506e0 >> DR0: DR1: DR2: >> DR3: DR6: fffe0ff0 DR7: 0400 >> Call Trace: >> btrfs_close_devices+0x8e/0x4b0 fs/btrfs/volumes.c:1184 >> open_ctree+0x492a/0x49cf fs/btrfs/disk-io.c:3381 >> btrfs_fill_super fs/btrfs/super.c:1316 [inline] >> btrfs_mount_root.cold+0x14/0x165 fs/btrfs/super.c:1672 >> legacy_get_tree+0x105/0x220 fs/fs_context.c:592 >> vfs_get_tree+0x89/0x2f0 fs/super.c:1547 >> fc_mount fs/namespace.c:983 [inline] >> vfs_kern_mount.part.0+0xd3/0x170 fs/namespace.c:1013 >> vfs_kern_mount+0x3c/0x60 fs/namespace.c:1000 >> btrfs_mount+0x234/0xaa0 fs/btrfs/super.c:1732 >> legacy_get_tree+0x105/0x220 fs/fs_context.c:592 >> vfs_get_tree+0x89/0x2f0 fs/super.c:1547 >> do_new_mount fs/namespace.c:2896 [inline] >> path_mount+0x12ae/0x1e70 fs/namespace.c:3216 >> do_mount fs/namespace.c:3229 [inline] >> __do_sys_mount fs/namespace.c:3437 [inline] >> __se_sys_mount fs/namespace.c:3414 [inline] >> __x64_sys_mount+0x27f/0x300 fs/namespace.c:3414 >> do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 >> entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> RIP: 0033:0x44851a >> Code: b8 08 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 cd a2 fb ff c3 66 2e 0f >> 1f 84 00 00 00 00 00 66 90 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff >> 0f 83 aa a2 fb ff c3 66 0f 1f 84 00 00 00 00 00 >> RSP: 002b:7ffcb26bce08 EFLAGS: 0293 ORIG_RAX: 00a5 >> RAX: ffda RBX: 0004 RCX: 0044851a >> RDX: 2000 RSI: 2100 RDI: 7ffcb26bce50 >> RBP: 7ffcb26bce90 R08: 7ffcb26bce90 R09: 2000 >> R10: R11: 0293 R12: 0003 >> R13: 7ffcb26bce50 R14: 0003 R15: 0001 >> > > > I am able to reproduce. And its quite strange at the moment. A devid 0 > is being scanned. Looks like crafted image. > > [19592.946397] BTRFS: device fsid f90cac8b-044b-4fa8-8bee-4b8d3da88dc2 > devid 0 transid 0 /dev/loop0 scanned by t (70902) Yeah if you look at the C reproducer it crafts an image in memory and then loop mounts it.
Re: [PATCH 3/9] fs: explicitly check for CHECK_IOVEC_ONLY in rw_copy_check_uvector
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 2/9] compat.h: fix a spelling error in
On 18/09/2020 14:48, Christoph Hellwig wrote: > We only have not compat_sys_readv64v2 syscall, only a We have no?
Re: kernel BUG at lib/string.c:LINE! (5)
On 16/09/2020 10:19, syzbot wrote:
> syzbot has bisected this issue to:
>
> commit 3951e7f050ac6a38bbc859fc3cd6093890c31d1c
> Author: Johannes Thumshirn
> Date: Mon Oct 7 09:11:01 2019 +
>
> btrfs: add xxhash64 to checksumming algorithms
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=10aadcc590
> start commit: e4c26faa Merge tag 'usb-5.9-rc5' of git://git.kernel.org/p..
> git tree: upstream
> final oops: https://syzkaller.appspot.com/x/report.txt?x=12aadcc590
> console output: https://syzkaller.appspot.com/x/log.txt?x=14aadcc590
> kernel config: https://syzkaller.appspot.com/x/.config?x=c61610091f4ca8c4
> dashboard link: https://syzkaller.appspot.com/bug?extid=e864a35d361e1d4e29a5
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=177582be90
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13deb2b590
>
> Reported-by: syzbot+e864a35d361e1d4e2...@syzkaller.appspotmail.com
> Fixes: 3951e7f050ac ("btrfs: add xxhash64 to checksumming algorithms")
>
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
>
This commit only allowed 8 byte checksums, but d5178578bcd4 ("btrfs: directly
call into crypto framework for checksumming") is the one that was missing the
conversion in btree_readpage_end_io_hook() and a prerequisite for 3951e7f050ac
("btrfs: add xxhash64 to checksumming algorithms"). So I think it makes more
sense to add d5178578bcd4 to the fixes line.
Re: first bad commit: [5795eb443060148796beeba106e4366d7f1458a6] scsi: sd_zbc: emulate ZONE_APPEND commands
On 12/09/2020 04:31, Damien Le Moal wrote:
> On 2020/09/12 8:07, Borislav Petkov wrote:
>> On Sat, Sep 12, 2020 at 12:17:59AM +0200, Borislav Petkov wrote:
>>> Enabling it, fixes the issue.
>>
>> Btw, I just hit the below warn with 5.8, while booting with the above
>> config option enabled. Looks familiar and I didn't trigger it with
>> 5.9-rc4+ so you guys either fixed it or something changed in-between:
>>
>> [5.124321] ata4.00: NCQ Send/Recv Log not supported
>> [5.131484] ata4.00: configured for UDMA/133
>> [5.135847] scsi 3:0:0:0: Direct-Access ATA ST8000AS0022-1WL
>> SN01 PQ: 0 ANSI: 5
>> [5.143972] sd 3:0:0:0: Attached scsi generic sg1 type 0
>> [5.144033] sd 3:0:0:0: [sdb] Host-aware zoned block device
>> [5.177105] sd 3:0:0:0: [sdb] 15628053168 512-byte logical blocks: (8.00
>> TB/7.28 TiB)
>> [5.184880] sd 3:0:0:0: [sdb] 4096-byte physical blocks
>> [5.190084] sd 3:0:0:0: [sdb] 29808 zones of 524288 logical blocks + 1
>> runt zone
>> [5.197439] sd 3:0:0:0: [sdb] Write Protect is off
>> [5.202220] sd 3:0:0:0: [sdb] Mode Sense: 00 3a 00 00
>> [5.207260] sd 3:0:0:0: [sdb] Write cache: enabled, read cache: enabled,
>> doesn't support DPO or FUA
>> [5.356631] sdb: sdb1
>> [5.359014] sdb: disabling host aware zoned block device support due to
>> partitions
>> [5.389941] [ cut here ]
>> [5.394557] WARNING: CPU: 8 PID: 164 at block/blk-settings.c:236
>> blk_queue_max_zone_append_sectors+0x12/0x40
>> [5.404300] Modules linked in:
>> [5.407365] CPU: 8 PID: 164 Comm: kworker/u32:6 Not tainted 5.8.0 #7
>> [5.413682] Hardware name: Micro-Star International Co., Ltd.
>> MS-7B79/X470 GAMING PRO (MS-7B79), BIOS 1.70 01/23/2019
>> [5.424191] Workqueue: events_unbound async_run_entry_fn
>> [5.429482] RIP: 0010:blk_queue_max_zone_append_sectors+0x12/0x40
>> [5.435543] Code: fe 0f 00 00 53 48 89 fb 0f 86 3d 07 00 00 48 89 b3 e0
>> 03 00 00 5b c3 90 0f 1f 44 00 00 8b 87 40 04 00 00 ff c8 83 f8 01 76 03 <0f>
>> 0b c3 8b 87 f8 03 00 00 39 87 f0 03 00 00 0f 46 87 f0 03 00 00
>> [5.454099] RSP: 0018:c9697c60 EFLAGS: 00010282
>> [5.459306] RAX: RBX: 8887fa0a9400 RCX:
>>
>> [5.466390] RDX: 8887faf0d400 RSI: 0540 RDI:
>> 8887f0dde6c8
>> [5.473474] RBP: 7471 R08: 001d1c40 R09:
>> 8887fee29ad0
>> [5.480559] R10: 0001434bac00 R11: 00358275 R12:
>> 0008
>> [5.487643] R13: 8887f0dde6c8 R14: 8887fa0a9738 R15:
>>
>> [5.494726] FS: () GS:8887fee0()
>> knlGS:
>> [5.502757] CS: 0010 DS: ES: CR0: 80050033
>> [5.508474] CR2: CR3: 02209000 CR4:
>> 003406e0
>> [5.515558] Call Trace:
>> [5.518026] sd_zbc_read_zones+0x323/0x480
>> [5.522122] sd_revalidate_disk+0x122b/0x2000
>> [5.526472] ? __device_add_disk+0x2f7/0x4e0
>> [5.530738] sd_probe+0x347/0x44b
>> [5.534058] really_probe+0x2c4/0x3f0
>> [5.537720] driver_probe_device+0xe1/0x150
>> [5.541902] ? driver_allows_async_probing+0x50/0x50
>> [5.546852] bus_for_each_drv+0x6a/0xa0
>> [5.550683] __device_attach_async_helper+0x8c/0xd0
>> [5.47] async_run_entry_fn+0x4a/0x180
>> [5.559636] process_one_work+0x1a5/0x3a0
>> [5.563637] worker_thread+0x50/0x3a0
>> [5.567300] ? process_one_work+0x3a0/0x3a0
>> [5.571480] kthread+0x117/0x160
>> [5.574715] ? kthread_park+0x90/0x90
>> [5.578377] ret_from_fork+0x22/0x30
>> [5.581960] ---[ end trace 94141003236730cf ]---
>> [5.586578] sd 3:0:0:0: [sdb] Attached SCSI disk
>> [6.186783] ata5: failed to resume link (SControl 0)
>> [6.191818] ata5: SATA link down (SStatus 0 SControl 0)
>>
This looks like we're trying to configure zone append max sectors
on a device that doesn't have the zoned flag set.
>
> Can you try this:
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 95018e650f2d..620539162ef1 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2968,8 +2968,13 @@ static void sd_read_block_characteristics(struct
> scsi_disk *sdkp)
> } else {
> sdkp->zoned = (buffer[8] >> 4) & 3;
> if (sdkp->zoned == 1 && !disk_has_partitions(sdkp->disk)) {
> +#ifdef CONFIG_BLK_DEV_ZONED
> /* Host-aware */
> q->limits.zoned = BLK_ZONED_HA;
> +#else
> + /* Host-aware drive is treated as a regular disk */
> + q->limits.zoned = BLK_ZONED_NONE;
> +#endif
> } else {
> /*
> * Treat drive-managed devices and host-aware devices
> @@ -3404,12 +3409,12 @@ static int sd_probe(struct device *dev)
> sdkp->first_scan = 1;
>
Re: first bad commit: [5795eb443060148796beeba106e4366d7f1458a6] scsi: sd_zbc: emulate ZONE_APPEND commands
On 12/09/2020 00:22, Randy Dunlap wrote:
> On 9/11/20 3:17 PM, Borislav Petkov wrote:
>> On Fri, Sep 11, 2020 at 09:53:12PM +0200, Borislav Petkov wrote:
>>> Now, looking at that patch:
>>>
>>> 5795eb443060 ("scsi: sd_zbc: emulate ZONE_APPEND commands")
>>>
>>> yeah, that doesn't revert cleanly. But it talks about zoned-something
>>> devices and that rings a bell because you guys broke my zoned device
>>> once already:
>>
>> Ok, so Johannes and I poked a bit on IRC and here it is:
>>
>> # CONFIG_BLK_DEV_ZONED is not set.
>>
>> Enabling it, fixes the issue.
>>
>
> Uh, you are not saying that enabling that CONFIG_ is the final fix, are you?
>
> If so, do I need to enable it, even if I don't have a zoned block device?
>
No he does have a zoned block device and no this is not the final fix, I
think one of the stubbed out functions is broken, but it's midnight here
so we're calling it a day and chime back in on Monday.
And this setup is a bit special, as Boris is using partitions on a host-aware
zoned block device which is somewhat exotic (see add_partition()).
Byte,
Johannes
Re: [PATCH -next] btrfs: Remove unused function calc_global_rsv_need_space()
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH] block: Fix potential page reference leak in __bio_iov_append_get_pages()
On 05/09/2020 11:41, Miaohe Lin wrote: > When bio_add_hw_page() failed, we left page reference still held in pages. I'd add "from iov_iter_get_pages()" to the above sentence. Otherwise Reviewed-by: Johannes Thumshirn
Re: [PATCH 04/19] floppy: use bdev_check_media_change
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH v3] null_blk: add support for max open/active zone limit for zoned devices
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 19/19] block: remove check_disk_change
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 18/19] sr: simplify sr_block_revalidate_disk
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 17/19] sr: use bdev_check_media_change
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 16/19] sd: use bdev_check_media_change
On 02/09/2020 16:21, Christoph Hellwig wrote: > call cd_revalidate_disk manually. As sd also calls sd_revalidate_disk ^~ sd_revalidate_disk Otherwise, Reviewed-by: Johannes Thumshirn
Re: [PATCH 15/19] md: use bdev_check_media_change
On 02/09/2020 16:16, Christoph Hellwig wrote: > The pcd driver does not have a ->revalidate_disk method, so it can just md ~^ Otherwise looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 13/19] ide-cd: remove idecd_revalidate_disk
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 12/19] ide-cd: use bdev_check_media_changed
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 11/19] gdrom: use bdev_check_media_change
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 10/19] paride/pcd: use bdev_check_media_change
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 08/19] xsysace: use bdev_check_media_change
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 07/19] swim3: use bdev_check_media_changed
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 02/19] amiflop: use bdev_check_media_change
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 06/19] swim: simplify media change handling
And down by one, Reviewed-by: Johannes Thumshirn
Re: [PATCH 05/19] swim: use bdev_check_media_change
Looks good, Reviewed-by: Johannes Thumshirn > +static int floppy_revalidate(struct gendisk *disk); Completely unrelated to this series but, this is the 3rd floppy driver in the series defining it's own floppy_revalidate() and naming it floppy_revalidate(). This makes grepping and reviewing a pain.
Re: [PATCH 04/19] floppy: use bdev_check_media_change
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 03/19] ataflop: use bdev_check_media_change
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 01/19] block: add a bdev_check_media_change helper
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 9/9] block: remove revalidate_disk()
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 6/9] nvme: opencode revalidate_disk in nvme_validate_ns
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 7/9] sd: open code revalidate_disk
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 5/9] block: use revalidate_disk_size in set_capacity_revalidate_and_notify
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 1/9] Documentation/filesystems/locking.rst: remove an incorrect sentence
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 3/9] block: rename bd_invalidated
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 4/9] block: add a new revalidate_disk_size helper
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH] block: tolerate 0 byte discard_granularity in __blkdev_issue_discard()
On 04/08/2020 16:45, Coly Li wrote: > Yes, Ming just posts a patch with a very similar change to loop device > driver. Ah ok. I'll go and have a look at Ming's patch then.
Re: [PATCH] block: tolerate 0 byte discard_granularity in __blkdev_issue_discard()
On 04/08/2020 16:23, Coly Li wrote: > This is the procedure to reproduce the panic, > # modprobe scsi_debug delay=0 dev_size_mb=2048 max_queue=1 > # losetup -f /dev/nvme0n1 --direct-io=on > # blkdiscard /dev/loop0 -o 0 -l 0x200 losetup -f /dev/sdX isn't it?
Re: [PATCH] block: tolerate 0 byte discard_granularity in __blkdev_issue_discard()
On 04/08/2020 16:37, Johannes Thumshirn wrote: > On 04/08/2020 16:34, Coly Li wrote: >> On 2020/8/4 22:31, Johannes Thumshirn wrote: >>> On 04/08/2020 16:23, Coly Li wrote: >>>> This is the procedure to reproduce the panic, >>>> # modprobe scsi_debug delay=0 dev_size_mb=2048 max_queue=1 >>>> # losetup -f /dev/nvme0n1 --direct-io=on >>>> # blkdiscard /dev/loop0 -o 0 -l 0x200 >>> >>> losetup -f /dev/sdX isn't it? >>> >> >> In my case, I use a NVMe SSD as the backing device of the loop device. >> Because I don't have a scsi lun. >> >> And loading scsi_debug module seems necessary, otherwise the discard >> process just hang and I cannot see the kernel panic (I don't know why yet). > > OK, now that's highly interesting. Does it also happen if you back loop with > a file? loop_config_discard() has different cases for the different backing > devices/files. S > Damn I didn't want to hit sent Does this (untested) change make a difference: diff --git a/drivers/block/loop.c b/drivers/block/loop.c index 475e1a738560..8a07a89d702e 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -895,6 +895,9 @@ static void loop_config_discard(struct loop_device *lo) blk_queue_max_write_zeroes_sectors(q, backingq->limits.max_write_zeroes_sectors); + q->limits.discard_granularity = + backingq->limits.discard_granularity; + /* * We use punch hole to reclaim the free space used by the * image a.k.a. discard. However we do not support discard if
Re: [PATCH] block: tolerate 0 byte discard_granularity in __blkdev_issue_discard()
On 04/08/2020 16:34, Coly Li wrote: > On 2020/8/4 22:31, Johannes Thumshirn wrote: >> On 04/08/2020 16:23, Coly Li wrote: >>> This is the procedure to reproduce the panic, >>> # modprobe scsi_debug delay=0 dev_size_mb=2048 max_queue=1 >>> # losetup -f /dev/nvme0n1 --direct-io=on >>> # blkdiscard /dev/loop0 -o 0 -l 0x200 >> >> losetup -f /dev/sdX isn't it? >> > > In my case, I use a NVMe SSD as the backing device of the loop device. > Because I don't have a scsi lun. > > And loading scsi_debug module seems necessary, otherwise the discard > process just hang and I cannot see the kernel panic (I don't know why yet). OK, now that's highly interesting. Does it also happen if you back loop with a file? loop_config_discard() has different cases for the different backing devices/files. S
Re: [PATCH 14/14] bdi: replace BDI_CAP_NO_{WRITEBACK,ACCT_DIRTY} with a single flag
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 12/14] bdi: replace BDI_CAP_STABLE_WRITES with a queue and a sb flag
Reviewed-by: Johannes Thumshirn
Re: [PATCH 13/14] bdi: invert BDI_CAP_NO_ACCT_WB
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 11/14] mm: use SWP_SYNCHRONOUS_IO more intelligently
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 10/14] bdi: remove BDI_CAP_SYNCHRONOUS_IO
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 06/14] block: lift setting the readahead size into the block layer
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 05/14] md: update the optimal I/O size on reshape
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 04/14] bdi: initialize ->ra_pages in bdi_init
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 02/14] drbd: remove dead code in device_to_statistics
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 09/14] bdi: remove BDI_CAP_CGROUP_WRITEBACK
On 22/07/2020 08:27, Christoph Hellwig wrote: > it is know to support cgroup writeback, or the bdi comes from the block knwon ~^ Apart from that, Reviewed-by: Johannes Thumshirn
Re: [PATCH 08/14] block: add helper macros for queue sysfs entries
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 07/14] block: make QUEUE_SYSFS_BIT_FNS a little more useful
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 02/14] drbd: remove dead code in device_to_statistics
On 22/07/2020 09:07, Christoph Hellwig wrote: > As far as I can tell this is a netlink user ABI. > I guess it has to stay then
Re: [PATCH 06/14] block: lift setting the readahead size into the block layer
On 22/07/2020 08:27, Christoph Hellwig wrote: > + q->backing_dev_info->ra_pages = > + max(queue_io_opt(q) * 2 / PAGE_SIZE, VM_READAHEAD_PAGES); Dumb question, wouldn't a '>> PAGE_SHIFT' be better instead of a potentially costly division? Or aren't we caring at all as it's a) not in the fast-path and b) compilers can optimize it to a shift?
Re: [PATCH 03/14] drbd: remove RB_CONGESTED_REMOTE
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 02/14] drbd: remove dead code in device_to_statistics
On 22/07/2020 08:28, Christoph Hellwig wrote: > Ever since the switch to blk-mq, a lower device not use by VM in-use/used? ~^ Also this looks like the last user of 'dev_lower_blocked' so it could be removed from device_statistics if it's not an ABI (not sure with this netlink stuff).
Re: [PATCH 01/14] fs: remove the unused SB_I_MULTIROOT flag
A little bit of git archeology shows the last user of SB_I_MULTIROOT is gone
with
f2aedb713c28 ("NFS: Add fs_context support.")
Reviewed-by: Johannes Thumshirn
Re: [PATCH v3 1/2] block: change REQ_OP_ZONE_RESET and REQ_OP_ZONE_RESET_ALL to be odd numbers
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH v3 2/2] block: add max_active_zones to blk-sysfs
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH v3 1/2] block: add max_open_zones to blk-sysfs
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH -next] scsi: sd_zbc: Remove unused inline functions
On 15/07/2020 04:56, YueHaibing wrote:
> They are never used, so can remove it.
>
> Signed-off-by: YueHaibing
> ---
> drivers/scsi/sd.h | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 3a74f4b45134..27c0f4e9b1d4 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -229,17 +229,11 @@ blk_status_t sd_zbc_prepare_zone_append(struct
> scsi_cmnd *cmd, sector_t *lba,
>
> #else /* CONFIG_BLK_DEV_ZONED */
>
> -static inline int sd_zbc_init(void)
> -{
> - return 0;
> -}
> -
> static inline int sd_zbc_init_disk(struct scsi_disk *sdkp)
> {
> return 0;
> }
>
> -static inline void sd_zbc_exit(void) {}
> static inline void sd_zbc_release_disk(struct scsi_disk *sdkp) {}
>
> static inline int sd_zbc_read_zones(struct scsi_disk *sdkp,
>
Woops that looks like some leftover from development. My bad.
Reviewed-by: Johannes Thumshirn
Re: [PATCH 13/30] scsi: libfc: fc_rport: Fix bitrotted function parameter and copy/paste error
On 08/07/2020 14:04, Lee Jones wrote: > Description should state 'remote' port, not 'local'. > > Fixes the following W=1 kernel build warning(s): > > drivers/scsi/libfc/fc_rport.c:1452: warning: Function parameter or member > 'rdata_arg' not described in 'fc_rport_logo_resp' > drivers/scsi/libfc/fc_rport.c:1452: warning: Excess function parameter > 'rport_arg' description in 'fc_rport_logo_resp' > > Cc: Hannes Reinecke > Signed-off-by: Lee Jones > --- > drivers/scsi/libfc/fc_rport.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c > index ea99e69d4d89c..18663a82865f9 100644 > --- a/drivers/scsi/libfc/fc_rport.c > +++ b/drivers/scsi/libfc/fc_rport.c > @@ -1445,7 +1445,7 @@ static void fc_rport_recv_rtv_req(struct fc_rport_priv > *rdata, > * fc_rport_logo_resp() - Handler for logout (LOGO) responses > * @sp: The sequence the LOGO was on > * @fp: The LOGO response frame > - * @rport_arg: The local port > + * @rdata_arg: The remote port > */ > static void fc_rport_logo_resp(struct fc_seq *sp, struct fc_frame *fp, > void *rdata_arg) > Please fold this into patch 11
Re: [PATCH 11/30] scsi: libfc: fc_rport: Fix a couple of misdocumented function parameters
On 08/07/2020 14:04, Lee Jones wrote: > @@ -1445,7 +1445,7 @@ static void fc_rport_recv_rtv_req(struct fc_rport_priv > *rdata, > * fc_rport_logo_resp() - Handler for logout (LOGO) responses > * @sp: The sequence the LOGO was on > * @fp: The LOGO response frame > - * @lport_arg: The local port > + * @rport_arg: The local port > */ > static void fc_rport_logo_resp(struct fc_seq *sp, struct fc_frame *fp, > void *rdata_arg) That doesn't look correct, s/rport_arg/rdata_arg/ and an rport is a remote port not a local port.
Re: [PATCH 03/30] scsi: libfc: fc_disc: trivial: Fix spelling mistake of 'discovery'
On 08/07/2020 14:12, Lee Jones wrote: > Obviously I'd be okay with that, but it will depend on whether his > tree is able to be rebased. Many public trees are unrebasable (if > that's a word). Yeah but in this early stage SCSI usually is re-baseable.
Re: [PATCH 03/30] scsi: libfc: fc_disc: trivial: Fix spelling mistake of 'discovery'
Looks good, Reviewed-by: Johannes Thumshirn I think Martin can fold this one into the original one
Re: [PATCH 09/10] scsi: libfc: fc_disc: Fix-up some incorrectly referenced function parameters
On 07/07/2020 16:01, Lee Jones wrote: > + * @disc: The descovery context s/descovery/discovery
Re: [PATCH v2 1/2] block: add max_open_zones to blk-sysfs
On 03/07/2020 14:09, Johannes Thumshirn wrote:
> On 03/07/2020 11:23, Niklas Cassel wrote:
>> On Fri, Jul 03, 2020 at 08:22:45AM +, Johannes Thumshirn wrote:
>>> On 02/07/2020 20:20, Niklas Cassel wrote:
>>>> Documentation/block/queue-sysfs.rst | 7 +++
>>>> block/blk-sysfs.c | 15 +++
>>>> drivers/nvme/host/zns.c | 1 +
>>>> drivers/scsi/sd_zbc.c | 4
>>>> include/linux/blkdev.h | 16
>>>> 5 files changed, 43 insertions(+)
>>>
>>> Sorry I haven't noticed before, but you forgot null_blk.
>>
>> Hello Johannes,
>>
>> Actually, I haven't forgotten about null_blk :)
>>
>> The problem with null_blk is that, compared to these simple patches that
>> simply exposes the Max Open Zones/Max Active Zones, null_blk additionally
>> has to keep track of all the zone accounting, and give an error if any
>> of these limits are exceeded.
>>
>> null_blk right now follows neither the ZBC nor the ZNS specification
>> (even though it is almost compliant with ZBC). However, since null_blk
>> is really great to test with, we want it to support Max Active Zones,
>> even if that concept does not exist in the ZBC standard.
>>
>> To add to the problem, open() does not work exactly the same in ZBC and
>> ZNS. In ZBC, the device always tries to close an implicit zone to make
>> room for an explicit zone. In ZNS, a controller that doesn't do this is
>> fully compliant with the spec.
>>
>> So now for null_blk, you have things like zones being implicit closed when
>> a new zone is opened. And now we will have an additional limit (Max Active
>> Zones), that we need to consider before we can even try to close a zone.
>>
>>
>> I've spent a couple of days trying to implement this already, and I think
>> that I have a way forward. However, considering that vacations are coming
>> up, and that I have a bunch of other stuff that I need to do before then,
>> I'm not 100% sure that I will be able to finish it in time for the coming
>> merge window.
>>
>> Therefore, I was hoping that we could merge this series as is, and I will
>> send out the null_blk changes when they are ready, which might or might
>> not make it for this merge window.
>
> No problem, I'm just working on MOR support for zonefs and though about how
> I'm going to test it. This is where I've noticed null_blk doesn't really
> expose a config knob for MOR. I can do some temporary hacks to test my changes
> and wait for your's to materialize.
>
>
>> In the meantime, MAR/MOR properties for null_blk will be exposed as 0,
>> which means "no limit". (Which is the case when a zoned block device driver
>> doesn't do an explicit call to blk_queue_max_{open,active}_zones()).
>
>
Another thing I've noticed, can you please introduce the bdev_max_open_zones()
and bdev_max_open_reagions() functions as well?
Like this (untested):
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index bb9e6eb6a7e6..612f4e36828d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1522,6 +1522,15 @@ static inline sector_t bdev_zone_sectors(struct
block_device *bdev)
return 0;
}
+static inline unsigned int bdev_max_open_zones(struct block_device *bdev)
+{
+ struct request_queue *q = bdev_get_queue(bdev);
+
+ if (q)
+ return queue_max_open_zones(q);
+ return 0;
+}
+
static inline int queue_dma_alignment(const struct request_queue *q)
{
return q ? q->dma_alignment : 511;
Re: [PATCH 2/2] block: enable zone-append for iov_iter of bvec type
On 03/07/2020 08:56, Kanchan Joshi wrote: [...] > Yes, zonefs does not use bvec iter. But while enabling io-uring path for > zone-append, I hit into this condition returning -EINVAL. > > Reference (from user zone-append series cover letter): > "Append using io_uring fixed-buffer ---> > This is flagged as not-supported at the moment. Reason being, for fixed-buffer > io-uring sends iov_iter of bvec type. But current append-infra in block-layer > does not support such iov_iter." > > And zone-append doesn't have a problem in using bvec iter as well, so > thought that this may make infra more complete/future-proof? As long as it's no problem for current in-tree users please keep it as is. Please submit this patch together with your io_uring series as a preparatory patch.
Re: [PATCH v2 1/2] block: add max_open_zones to blk-sysfs
On 03/07/2020 11:23, Niklas Cassel wrote:
> On Fri, Jul 03, 2020 at 08:22:45AM +0000, Johannes Thumshirn wrote:
>> On 02/07/2020 20:20, Niklas Cassel wrote:
>>> Documentation/block/queue-sysfs.rst | 7 +++
>>> block/blk-sysfs.c | 15 +++
>>> drivers/nvme/host/zns.c | 1 +
>>> drivers/scsi/sd_zbc.c | 4
>>> include/linux/blkdev.h | 16
>>> 5 files changed, 43 insertions(+)
>>
>> Sorry I haven't noticed before, but you forgot null_blk.
>
> Hello Johannes,
>
> Actually, I haven't forgotten about null_blk :)
>
> The problem with null_blk is that, compared to these simple patches that
> simply exposes the Max Open Zones/Max Active Zones, null_blk additionally
> has to keep track of all the zone accounting, and give an error if any
> of these limits are exceeded.
>
> null_blk right now follows neither the ZBC nor the ZNS specification
> (even though it is almost compliant with ZBC). However, since null_blk
> is really great to test with, we want it to support Max Active Zones,
> even if that concept does not exist in the ZBC standard.
>
> To add to the problem, open() does not work exactly the same in ZBC and
> ZNS. In ZBC, the device always tries to close an implicit zone to make
> room for an explicit zone. In ZNS, a controller that doesn't do this is
> fully compliant with the spec.
>
> So now for null_blk, you have things like zones being implicit closed when
> a new zone is opened. And now we will have an additional limit (Max Active
> Zones), that we need to consider before we can even try to close a zone.
>
>
> I've spent a couple of days trying to implement this already, and I think
> that I have a way forward. However, considering that vacations are coming
> up, and that I have a bunch of other stuff that I need to do before then,
> I'm not 100% sure that I will be able to finish it in time for the coming
> merge window.
>
> Therefore, I was hoping that we could merge this series as is, and I will
> send out the null_blk changes when they are ready, which might or might
> not make it for this merge window.
No problem, I'm just working on MOR support for zonefs and though about how
I'm going to test it. This is where I've noticed null_blk doesn't really
expose a config knob for MOR. I can do some temporary hacks to test my changes
and wait for your's to materialize.
> In the meantime, MAR/MOR properties for null_blk will be exposed as 0,
> which means "no limit". (Which is the case when a zoned block device driver
> doesn't do an explicit call to blk_queue_max_{open,active}_zones()).
Re: [PATCH v2 1/2] block: add max_open_zones to blk-sysfs
On 02/07/2020 20:20, Niklas Cassel wrote: > Documentation/block/queue-sysfs.rst | 7 +++ > block/blk-sysfs.c | 15 +++ > drivers/nvme/host/zns.c | 1 + > drivers/scsi/sd_zbc.c | 4 > include/linux/blkdev.h | 16 > 5 files changed, 43 insertions(+) Sorry I haven't noticed before, but you forgot null_blk.
Re: [PATCH 7/7] block: remove the all_bdevs list
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 6/7] block: remove the unused bd_private field from struct block_device
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 5/7] block: remove the bd_queue field from struct block_device
Looks good, Reviewed-by: Johannes Thumshirn
Re: [PATCH 4/7] block: remove the bd_block_size field from struct block_device
Looks good, Reviewed-by: Johannes Thumshirn
