From: Logan Gunthorpe <logang@deltatee.com> To: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org> Cc: Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>, Keith Busch <kbusch@kernel.org>, Jens Axboe <axboe@fb.com>, Max Gurtovoy <maxg@mellanox.com>, Stephen Bates <sbates@raithlin.com> Subject: Re: [PATCH v13 8/9] nvmet-passthru: Add enable/disable helpers Date: Thu, 11 Jun 2020 17:24:30 -0600 [thread overview] Message-ID: <078ffd88-d2f9-bfa1-5a81-97abe0b66653@deltatee.com> (raw) In-Reply-To: <BYAPR04MB496581334E89E2E3086C268286800@BYAPR04MB4965.namprd04.prod.outlook.com> On 2020-06-11 5:13 p.m., Chaitanya Kulkarni wrote: > On 5/14/20 10:23 AM, Logan Gunthorpe wrote: >> + if (subsys->ver < NVME_VS(1, 2, 1)) { >> + pr_warn("nvme controller version is too old: %d.%d.%d, advertising 1.2.1\n", > Is it more than 80 char ? can it be ? > pr_warn("nvme controller version is too old: "); > pr_warn("%d.%d.%d, advertising 1.2.1\n", Splitting printks is a bad idea because they won't necessarily end up in dmesg right next to each other -- other prints can be interleaved. Moreover, it breaks any tools that are dealing with kernel logs as single records (ie through /dev/kmsg). Continuations (or printks without a trailing "\n") would require using pr_cont() and generally that's discouraged. >> + (int)NVME_MAJOR(subsys->ver), >> + (int)NVME_MINOR(subsys->ver), >> + (int)NVME_TERTIARY(subsys->ver)); >> + subsys->ver = NVME_VS(1, 2, 1); >> + } > > NVMe blktests are running on QEMU based controller. This will generate > warning every-time. Yup. > Also, I didn't understand int type cast, I wonder under what condition > all these macros will return -ve values since ver of type u64 ? Yes, I'm not sure what I was thinking when I put in the cast. Probably better just to prints "%llu". I'll fix this for the next revision. Thanks, Logan
WARNING: multiple messages have this Message-ID (diff)
From: Logan Gunthorpe <logang@deltatee.com> To: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org> Cc: Sagi Grimberg <sagi@grimberg.me>, Stephen Bates <sbates@raithlin.com>, Jens Axboe <axboe@fb.com>, Keith Busch <kbusch@kernel.org>, Max Gurtovoy <maxg@mellanox.com>, Christoph Hellwig <hch@lst.de> Subject: Re: [PATCH v13 8/9] nvmet-passthru: Add enable/disable helpers Date: Thu, 11 Jun 2020 17:24:30 -0600 [thread overview] Message-ID: <078ffd88-d2f9-bfa1-5a81-97abe0b66653@deltatee.com> (raw) In-Reply-To: <BYAPR04MB496581334E89E2E3086C268286800@BYAPR04MB4965.namprd04.prod.outlook.com> On 2020-06-11 5:13 p.m., Chaitanya Kulkarni wrote: > On 5/14/20 10:23 AM, Logan Gunthorpe wrote: >> + if (subsys->ver < NVME_VS(1, 2, 1)) { >> + pr_warn("nvme controller version is too old: %d.%d.%d, advertising 1.2.1\n", > Is it more than 80 char ? can it be ? > pr_warn("nvme controller version is too old: "); > pr_warn("%d.%d.%d, advertising 1.2.1\n", Splitting printks is a bad idea because they won't necessarily end up in dmesg right next to each other -- other prints can be interleaved. Moreover, it breaks any tools that are dealing with kernel logs as single records (ie through /dev/kmsg). Continuations (or printks without a trailing "\n") would require using pr_cont() and generally that's discouraged. >> + (int)NVME_MAJOR(subsys->ver), >> + (int)NVME_MINOR(subsys->ver), >> + (int)NVME_TERTIARY(subsys->ver)); >> + subsys->ver = NVME_VS(1, 2, 1); >> + } > > NVMe blktests are running on QEMU based controller. This will generate > warning every-time. Yup. > Also, I didn't understand int type cast, I wonder under what condition > all these macros will return -ve values since ver of type u64 ? Yes, I'm not sure what I was thinking when I put in the cast. Probably better just to prints "%llu". I'll fix this for the next revision. Thanks, Logan _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2020-06-11 23:24 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-14 17:22 [PATCH v13 0/9] nvmet: add target passthru commands support Logan Gunthorpe 2020-05-14 17:22 ` Logan Gunthorpe 2020-05-14 17:22 ` [PATCH v13 1/9] nvme-core: Clear any SGL flags in passthru commands Logan Gunthorpe 2020-05-14 17:22 ` Logan Gunthorpe 2020-05-14 17:22 ` [PATCH v13 2/9] nvme: Create helper function to obtain command effects Logan Gunthorpe 2020-05-14 17:22 ` Logan Gunthorpe 2020-06-11 22:56 ` Chaitanya Kulkarni 2020-06-11 22:56 ` Chaitanya Kulkarni 2020-06-11 23:09 ` Logan Gunthorpe 2020-06-11 23:09 ` Logan Gunthorpe 2020-05-14 17:22 ` [PATCH v13 3/9] nvme: Move nvme_passthru_[start|end]() calls to common helper Logan Gunthorpe 2020-05-14 17:22 ` Logan Gunthorpe 2020-05-14 17:22 ` [PATCH v13 4/9] nvmet-passthru: Introduce NVMet passthru Kconfig option Logan Gunthorpe 2020-05-14 17:22 ` Logan Gunthorpe 2020-05-14 17:22 ` [PATCH v13 5/9] nvme-core: Introduce nvme_ctrl_get_by_path() Logan Gunthorpe 2020-05-14 17:22 ` Logan Gunthorpe 2020-06-11 23:02 ` Chaitanya Kulkarni 2020-06-11 23:02 ` Chaitanya Kulkarni 2020-06-11 23:09 ` Logan Gunthorpe 2020-06-11 23:09 ` Logan Gunthorpe 2020-05-14 17:22 ` [PATCH v13 6/9] nvme: Export existing nvme core functions Logan Gunthorpe 2020-05-14 17:22 ` Logan Gunthorpe 2020-05-14 17:22 ` [PATCH v13 7/9] nvmet-passthru: Add passthru code to process commands Logan Gunthorpe 2020-05-14 17:22 ` Logan Gunthorpe 2020-06-11 23:45 ` Chaitanya Kulkarni 2020-06-11 23:45 ` Chaitanya Kulkarni 2020-06-11 23:46 ` Logan Gunthorpe 2020-06-11 23:46 ` Logan Gunthorpe 2020-05-14 17:22 ` [PATCH v13 8/9] nvmet-passthru: Add enable/disable helpers Logan Gunthorpe 2020-05-14 17:22 ` Logan Gunthorpe 2020-06-11 23:05 ` Chaitanya Kulkarni 2020-06-11 23:05 ` Chaitanya Kulkarni 2020-06-11 23:14 ` Logan Gunthorpe 2020-06-11 23:14 ` Logan Gunthorpe 2020-06-11 23:32 ` Chaitanya Kulkarni 2020-06-11 23:32 ` Chaitanya Kulkarni 2020-06-11 23:13 ` Chaitanya Kulkarni 2020-06-11 23:13 ` Chaitanya Kulkarni 2020-06-11 23:24 ` Logan Gunthorpe [this message] 2020-06-11 23:24 ` Logan Gunthorpe 2020-05-14 17:22 ` [PATCH v13 9/9] nvmet-configfs: Introduce passthru configfs interface Logan Gunthorpe 2020-05-14 17:22 ` Logan Gunthorpe 2020-06-11 19:16 ` [PATCH v13 0/9] nvmet: add target passthru commands support Chaitanya Kulkarni 2020-06-11 19:16 ` Chaitanya Kulkarni 2020-06-11 19:19 ` Logan Gunthorpe 2020-06-11 19:19 ` Logan Gunthorpe
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=078ffd88-d2f9-bfa1-5a81-97abe0b66653@deltatee.com \ --to=logang@deltatee.com \ --cc=Chaitanya.Kulkarni@wdc.com \ --cc=axboe@fb.com \ --cc=hch@lst.de \ --cc=kbusch@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvme@lists.infradead.org \ --cc=maxg@mellanox.com \ --cc=sagi@grimberg.me \ --cc=sbates@raithlin.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.