Message ID | 1544595853-18492-1-git-send-email-amhetre@nvidia.com |
---|---|
State | Changes Requested |
Headers | show |
Series | scatterlist: Update size type to support greater then 4GB size. | expand |
From: Ashish Mhetre <amhetre@nvidia.com> Date: Wed, 12 Dec 2018 11:54:13 +0530 > diff --git a/include/linux/nvme.h b/include/linux/nvme.h > index 68e91ef..0a07a29 100644 > --- a/include/linux/nvme.h > +++ b/include/linux/nvme.h > @@ -587,7 +587,7 @@ enum { > > struct nvme_sgl_desc { > __le64 addr; > - __le32 length; > + __le64 length; > __u8 rsvd[3]; > __u8 type; > }; Isn't this a device or protocol defined datastructure? You can't just change it like this.
> struct nvme_sgl_desc { > __le64 addr; > - __le32 length; > + __le64 length; > __u8 rsvd[3]; > __u8 type; > }; in what world changing a wire protocol for this make sense? please get rid of this hunk, NVMe will never cross the 32 bit sg element size.
>> struct nvme_sgl_desc { >> __le64 addr; >> - __le32 length; >> + __le64 length; >> __u8 rsvd[3]; >> __u8 type; >> }; > > Isn't this a device or protocol defined datastructure? You can't just > change it like this. You're correct, we can't... [Replied before seeing this issue was already highlighted] The positive side is that it can safely be removed without affecting the rest of the patch...
On 12/12/18 12:19 PM, Sagi Grimberg wrote: > >>> struct nvme_sgl_desc { >>> __le64 addr; >>> - __le32 length; >>> + __le64 length; >>> __u8 rsvd[3]; >>> __u8 type; >>> }; >> >> Isn't this a device or protocol defined datastructure? You can't just >> change it like this. > > You're correct, we can't... > [Replied before seeing this issue was already highlighted] > > The positive side is that it can safely be removed without affecting the > rest of the patch... Ohh, I am not aware of this protocol defined data-structures. But it seems that this need not be changed as Sagi is saying sg length for NVME will never cross 32 bit size. I'll send a new version removing this change. Thanks.
On Wed, 12 Dec 2018 at 07:25, Ashish Mhetre <amhetre@nvidia.com> wrote: > > From: Krishna Reddy <vdumpa@nvidia.com> > > In the cases where greater than 4GB allocations are required, current > definition of scatterlist doesn't support it. For example, Tegra devices > have more than 4GB of system memory available. So they are expected to > support larger allocation requests. > This patch updates the type of scatterlist members to support creating > scatterlist for allocations of size greater than 4GB size. Can you explain what the use case is? We have had systems with more than 4 GB for a while now, so where does this new requirement come from? Also, you are changing 'length' to size_t and 'offset' to unsigned long. What is the rationale behind that? Did you consider 32-bit architectures with PAE at all? > Updated the files that are necessary to fix compilation issues with updated > type of variables. > > With definition of scatterlist changed in this patch, size of sg has > increased from 28 bytes to 40 bytes because of which allocations in nvme > driver are crossing order-0( as sizeof(struct scatterlist) is used in nvme > driver allocations ) i.e. they are not able to fit into single page. > > Recently a patch to limit the nvme allocations to order-0 is mergerd. > 'commit 943e942e6266f22babee5efeb00f8f672fbff5bd ("nvme-pci: limit > max IO size and segments to avoid high order allocations")' > Because of that patch, WARN log is getting printed in our case as > definition of scatterlist has changed. Alloc size of nvme is calculated as > NVME_MAX_SEGS * sizeof(struct scatterlist). As sizeof(struct scatterlist) > has changed from 28 bytes to 40 bytes, so updating NVME_MAX_SEGS from 127 > to 88 to correspond to original nvme alloc size value. > > Signed-off-by: Krishna Reddy <vdumpa@nvidia.com> > Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> > --- > crypto/shash.c | 2 +- > drivers/ata/libata-sff.c | 2 +- > drivers/mmc/host/sdhci.c | 2 +- > drivers/mmc/host/toshsd.c | 2 +- > drivers/mmc/host/usdhi6rol0.c | 14 +++++++------- > drivers/nvme/host/pci.c | 8 ++++---- > include/linux/nvme.h | 2 +- > include/linux/scatterlist.h | 8 ++++---- > 8 files changed, 20 insertions(+), 20 deletions(-) > > diff --git a/crypto/shash.c b/crypto/shash.c > index d21f04d..434970e 100644 > --- a/crypto/shash.c > +++ b/crypto/shash.c > @@ -298,7 +298,7 @@ int shash_ahash_digest(struct ahash_request *req, struct shash_desc *desc) > > if (nbytes && > (sg = req->src, offset = sg->offset, > - nbytes < min(sg->length, ((unsigned int)(PAGE_SIZE)) - offset))) { > + nbytes < min(sg->length, ((size_t)(PAGE_SIZE)) - offset))) { > void *data; > > data = kmap_atomic(sg_page(sg)); > diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c > index c5ea0fc..675def6 100644 > --- a/drivers/ata/libata-sff.c > +++ b/drivers/ata/libata-sff.c > @@ -810,7 +810,7 @@ static int __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes) > offset %= PAGE_SIZE; > > /* don't overrun current sg */ > - count = min(sg->length - qc->cursg_ofs, bytes); > + count = min(sg->length - qc->cursg_ofs, (size_t)bytes); > > /* don't cross page boundaries */ > count = min(count, (unsigned int)PAGE_SIZE - offset); > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 99bdae5..bd84c0c 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1025,7 +1025,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd) > if (unlikely(length_mask | offset_mask)) { > for_each_sg(data->sg, sg, data->sg_len, i) { > if (sg->length & length_mask) { > - DBG("Reverting to PIO because of transfer size (%d)\n", > + DBG("Reverting to PIO because of transfer size (%zd)\n", > sg->length); > host->flags &= ~SDHCI_REQ_USE_DMA; > break; > diff --git a/drivers/mmc/host/toshsd.c b/drivers/mmc/host/toshsd.c > index dd961c5..af00936 100644 > --- a/drivers/mmc/host/toshsd.c > +++ b/drivers/mmc/host/toshsd.c > @@ -479,7 +479,7 @@ static void toshsd_start_data(struct toshsd_host *host, struct mmc_data *data) > { > unsigned int flags = SG_MITER_ATOMIC; > > - dev_dbg(&host->pdev->dev, "setup data transfer: blocksize %08x nr_blocks %d, offset: %08x\n", > + dev_dbg(&host->pdev->dev, "setup data transfer: blocksize %08x nr_blocks %d, offset: %08lx\n", > data->blksz, data->blocks, data->sg->offset); > > host->data = data; > diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c > index cd8b1b9..5fce5ff 100644 > --- a/drivers/mmc/host/usdhi6rol0.c > +++ b/drivers/mmc/host/usdhi6rol0.c > @@ -316,7 +316,7 @@ static void usdhi6_blk_bounce(struct usdhi6_host *host, > struct mmc_data *data = host->mrq->data; > size_t blk_head = host->head_len; > > - dev_dbg(mmc_dev(host->mmc), "%s(): CMD%u of %u SG: %ux%u @ 0x%x\n", > + dev_dbg(mmc_dev(host->mmc), "%s(): CMD%u of %u SG: %ux%u @ 0x%lx\n", > __func__, host->mrq->cmd->opcode, data->sg_len, > data->blksz, data->blocks, sg->offset); > > @@ -360,7 +360,7 @@ static void *usdhi6_sg_map(struct usdhi6_host *host) > > WARN(host->pg.page, "%p not properly unmapped!\n", host->pg.page); > if (WARN(sg_dma_len(sg) % data->blksz, > - "SG size %u isn't a multiple of block size %u\n", > + "SG size %zu isn't a multiple of block size %u\n", > sg_dma_len(sg), data->blksz)) > return NULL; > > @@ -383,7 +383,7 @@ static void *usdhi6_sg_map(struct usdhi6_host *host) > else > host->blk_page = host->pg.mapped; > > - dev_dbg(mmc_dev(host->mmc), "Mapped %p (%lx) at %p + %u for CMD%u @ 0x%p\n", > + dev_dbg(mmc_dev(host->mmc), "Mapped %p (%lx) at %p + %lu for CMD%u @ 0x%p\n", > host->pg.page, page_to_pfn(host->pg.page), host->pg.mapped, > sg->offset, host->mrq->cmd->opcode, host->mrq); > > @@ -492,7 +492,7 @@ static void usdhi6_sg_advance(struct usdhi6_host *host) > host->sg = next; > > if (WARN(next && sg_dma_len(next) % data->blksz, > - "SG size %u isn't a multiple of block size %u\n", > + "SG size %zu isn't a multiple of block size %u\n", > sg_dma_len(next), data->blksz)) > data->error = -EINVAL; > > @@ -1044,7 +1044,7 @@ static int usdhi6_rq_start(struct usdhi6_host *host) > (data->blksz % 4 || > data->sg->offset % 4)) > dev_dbg(mmc_dev(host->mmc), > - "Bad SG of %u: %ux%u @ %u\n", data->sg_len, > + "Bad SG of %u: %ux%u @ %lu\n", data->sg_len, > data->blksz, data->blocks, data->sg->offset); > > /* Enable DMA for USDHI6_MIN_DMA bytes or more */ > @@ -1056,7 +1056,7 @@ static int usdhi6_rq_start(struct usdhi6_host *host) > usdhi6_write(host, USDHI6_CC_EXT_MODE, USDHI6_CC_EXT_MODE_SDRW); > > dev_dbg(mmc_dev(host->mmc), > - "%s(): request opcode %u, %u blocks of %u bytes in %u segments, %s %s @+0x%x%s\n", > + "%s(): request opcode %u, %u blocks of %u bytes in %u segments, %s %s @+0x%lx%s\n", > __func__, cmd->opcode, data->blocks, data->blksz, > data->sg_len, use_dma ? "DMA" : "PIO", > data->flags & MMC_DATA_READ ? "read" : "write", > @@ -1704,7 +1704,7 @@ static void usdhi6_timeout_work(struct work_struct *work) > case USDHI6_WAIT_FOR_WRITE: > sg = host->sg ?: data->sg; > dev_dbg(mmc_dev(host->mmc), > - "%c: page #%u @ +0x%zx %ux%u in SG%u. Current SG %u bytes @ %u\n", > + "%c: page #%u @ +0x%zx %ux%u in SG%u. Current SG %zu bytes @ %lu\n", > data->flags & MMC_DATA_READ ? 'R' : 'W', host->page_idx, > host->offset, data->blocks, data->blksz, data->sg_len, > sg_dma_len(sg), sg->offset); > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c > index d668682..57ef89d 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -43,7 +43,7 @@ > * require an sg allocation that needs more than a page of data. > */ > #define NVME_MAX_KB_SZ 4096 > -#define NVME_MAX_SEGS 127 > +#define NVME_MAX_SEGS 88 > > static int use_threaded_interrupts; > module_param(use_threaded_interrupts, int, 0); > @@ -552,8 +552,8 @@ static void nvme_print_sgl(struct scatterlist *sgl, int nents) > > for_each_sg(sgl, sg, nents, i) { > dma_addr_t phys = sg_phys(sg); > - pr_warn("sg[%d] phys_addr:%pad offset:%d length:%d " > - "dma_address:%pad dma_length:%d\n", > + pr_warn("sg[%d] phys_addr:%pad offset:%lu length:%zu " > + "dma_address:%pad dma_length:%zu\n", > i, &phys, sg->offset, sg->length, &sg_dma_address(sg), > sg_dma_len(sg)); > } > @@ -566,7 +566,7 @@ static blk_status_t nvme_pci_setup_prps(struct nvme_dev *dev, > struct dma_pool *pool; > int length = blk_rq_payload_bytes(req); > struct scatterlist *sg = iod->sg; > - int dma_len = sg_dma_len(sg); > + u64 dma_len = sg_dma_len(sg); > u64 dma_addr = sg_dma_address(sg); > u32 page_size = dev->ctrl.page_size; > int offset = dma_addr & (page_size - 1); > diff --git a/include/linux/nvme.h b/include/linux/nvme.h > index 68e91ef..0a07a29 100644 > --- a/include/linux/nvme.h > +++ b/include/linux/nvme.h > @@ -587,7 +587,7 @@ enum { > > struct nvme_sgl_desc { > __le64 addr; > - __le32 length; > + __le64 length; > __u8 rsvd[3]; > __u8 type; > }; > diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h > index 093aa57..f6d3482 100644 > --- a/include/linux/scatterlist.h > +++ b/include/linux/scatterlist.h > @@ -10,11 +10,11 @@ > > struct scatterlist { > unsigned long page_link; > - unsigned int offset; > - unsigned int length; > + unsigned long offset; > + size_t length; > dma_addr_t dma_address; > #ifdef CONFIG_NEED_SG_DMA_LENGTH > - unsigned int dma_length; > + size_t dma_length; > #endif > }; > > @@ -114,7 +114,7 @@ static inline void sg_assign_page(struct scatterlist *sg, struct page *page) > * > **/ > static inline void sg_set_page(struct scatterlist *sg, struct page *page, > - unsigned int len, unsigned int offset) > + size_t len, unsigned long offset) > { > sg_assign_page(sg, page); > sg->offset = offset; > -- > 2.7.4 >
scatterlist elements longer than 4GB sound odd. Please submit it in a series with your actual user so that we can help figuring out if it really makes sense or if there is a better way to solve your problem. As is this patch will massively increase the memory usage for all users of struct scatterlist, so you better have a really good reason.
Hi Krishna, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.20-rc6 next-20181214] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Ashish-Mhetre/scatterlist-Update-size-type-to-support-greater-then-4GB-size/20181214-214801 config: x86_64-randconfig-x000-201849 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): In file included from include/linux/init.h:5:0, from drivers/nvme//host/fabrics.c:15: drivers/nvme//host/fabrics.c: In function 'nvmf_exit': >> include/linux/compiler.h:372:38: error: call to '__compiletime_assert_1149' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct nvmf_connect_command) != 64 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:353:4: note: in definition of macro '__compiletime_assert' prefix ## suffix(); \ ^~~~~~ include/linux/compiler.h:372:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~~~~~~~~~~~~~ >> drivers/nvme//host/fabrics.c:1149:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON(sizeof(struct nvmf_connect_command) != 64); ^~~~~~~~~~~~ -- In file included from include/linux/init.h:5:0, from drivers/nvme/host/fabrics.c:15: drivers/nvme/host/fabrics.c: In function 'nvmf_exit': >> include/linux/compiler.h:372:38: error: call to '__compiletime_assert_1149' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct nvmf_connect_command) != 64 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:353:4: note: in definition of macro '__compiletime_assert' prefix ## suffix(); \ ^~~~~~ include/linux/compiler.h:372:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~~~~~~~~~~~~~ drivers/nvme/host/fabrics.c:1149:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON(sizeof(struct nvmf_connect_command) != 64); ^~~~~~~~~~~~ vim +/__compiletime_assert_1149 +372 include/linux/compiler.h 9a8ab1c3 Daniel Santos 2013-02-21 358 9a8ab1c3 Daniel Santos 2013-02-21 359 #define _compiletime_assert(condition, msg, prefix, suffix) \ 9a8ab1c3 Daniel Santos 2013-02-21 360 __compiletime_assert(condition, msg, prefix, suffix) 9a8ab1c3 Daniel Santos 2013-02-21 361 9a8ab1c3 Daniel Santos 2013-02-21 362 /** 9a8ab1c3 Daniel Santos 2013-02-21 363 * compiletime_assert - break build and emit msg if condition is false 9a8ab1c3 Daniel Santos 2013-02-21 364 * @condition: a compile-time constant condition to check 9a8ab1c3 Daniel Santos 2013-02-21 365 * @msg: a message to emit if condition is false 9a8ab1c3 Daniel Santos 2013-02-21 366 * 9a8ab1c3 Daniel Santos 2013-02-21 367 * In tradition of POSIX assert, this macro will break the build if the 9a8ab1c3 Daniel Santos 2013-02-21 368 * supplied condition is *false*, emitting the supplied error message if the 9a8ab1c3 Daniel Santos 2013-02-21 369 * compiler has support to do so. 9a8ab1c3 Daniel Santos 2013-02-21 370 */ 9a8ab1c3 Daniel Santos 2013-02-21 371 #define compiletime_assert(condition, msg) \ 9a8ab1c3 Daniel Santos 2013-02-21 @372 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) 9a8ab1c3 Daniel Santos 2013-02-21 373 :::::: The code at line 372 was first introduced by commit :::::: 9a8ab1c39970a4938a72d94e6fd13be88a797590 bug.h, compiler.h: introduce compiletime_assert & BUILD_BUG_ON_MSG :::::: TO: Daniel Santos <daniel.santos@pobox.com> :::::: CC: Linus Torvalds <torvalds@linux-foundation.org> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Krishna, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.20-rc6 next-20181214] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Ashish-Mhetre/scatterlist-Update-size-type-to-support-greater-then-4GB-size/20181214-214801 config: x86_64-randconfig-x012-201849 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): In file included from include/linux/kernel.h:10:0, from include/linux/list.h:9, from include/linux/async.h:16, from drivers/nvme/host/pci.c:16: In function '_nvme_check_size', inlined from 'nvme_exit' at drivers/nvme/host/pci.c:2748:2: >> include/linux/compiler.h:372:38: error: call to '__compiletime_assert_206' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct nvme_rw_command) != 64 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:353:4: note: in definition of macro '__compiletime_assert' prefix ## suffix(); \ ^~~~~~ include/linux/compiler.h:372:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~~~~~~~~~~~~~ >> drivers/nvme/host/pci.c:206:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON(sizeof(struct nvme_rw_command) != 64); ^~~~~~~~~~~~ >> include/linux/compiler.h:372:38: error: call to '__compiletime_assert_210' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct nvme_features) != 64 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:353:4: note: in definition of macro '__compiletime_assert' prefix ## suffix(); \ ^~~~~~ include/linux/compiler.h:372:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~~~~~~~~~~~~~ drivers/nvme/host/pci.c:210:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON(sizeof(struct nvme_features) != 64); ^~~~~~~~~~~~ >> include/linux/compiler.h:372:38: error: call to '__compiletime_assert_213' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct nvme_command) != 64 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:353:4: note: in definition of macro '__compiletime_assert' prefix ## suffix(); \ ^~~~~~ include/linux/compiler.h:372:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~~~~~~~~~~~~~ drivers/nvme/host/pci.c:213:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON(sizeof(struct nvme_command) != 64); ^~~~~~~~~~~~ -- In file included from include/linux/kernel.h:10:0, from include/linux/list.h:9, from include/linux/async.h:16, from drivers/nvme//host/pci.c:16: In function '_nvme_check_size', inlined from 'nvme_exit' at drivers/nvme//host/pci.c:2748:2: >> include/linux/compiler.h:372:38: error: call to '__compiletime_assert_206' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct nvme_rw_command) != 64 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:353:4: note: in definition of macro '__compiletime_assert' prefix ## suffix(); \ ^~~~~~ include/linux/compiler.h:372:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~~~~~~~~~~~~~ drivers/nvme//host/pci.c:206:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON(sizeof(struct nvme_rw_command) != 64); ^~~~~~~~~~~~ >> include/linux/compiler.h:372:38: error: call to '__compiletime_assert_210' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct nvme_features) != 64 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:353:4: note: in definition of macro '__compiletime_assert' prefix ## suffix(); \ ^~~~~~ include/linux/compiler.h:372:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~~~~~~~~~~~~~ drivers/nvme//host/pci.c:210:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON(sizeof(struct nvme_features) != 64); ^~~~~~~~~~~~ >> include/linux/compiler.h:372:38: error: call to '__compiletime_assert_213' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct nvme_command) != 64 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:353:4: note: in definition of macro '__compiletime_assert' prefix ## suffix(); \ ^~~~~~ include/linux/compiler.h:372:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~~~~~~~~~~~~~~~~~ include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~~~~~~~~~~~~~ drivers/nvme//host/pci.c:213:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON(sizeof(struct nvme_command) != 64); ^~~~~~~~~~~~ vim +/BUILD_BUG_ON +206 drivers/nvme/host/pci.c b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 200 b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 201 /* b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 202 * Check we didin't inadvertently grow the command struct b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 203 */ b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 204 static inline void _nvme_check_size(void) b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 205 { b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 @206 BUILD_BUG_ON(sizeof(struct nvme_rw_command) != 64); b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 207 BUILD_BUG_ON(sizeof(struct nvme_create_cq) != 64); b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 208 BUILD_BUG_ON(sizeof(struct nvme_create_sq) != 64); b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 209 BUILD_BUG_ON(sizeof(struct nvme_delete_queue) != 64); b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 210 BUILD_BUG_ON(sizeof(struct nvme_features) != 64); f8ebf840 drivers/block/nvme-core.c Vishal Verma 2013-03-27 211 BUILD_BUG_ON(sizeof(struct nvme_format_cmd) != 64); c30341dc drivers/block/nvme-core.c Keith Busch 2013-12-10 212 BUILD_BUG_ON(sizeof(struct nvme_abort_cmd) != 64); b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 213 BUILD_BUG_ON(sizeof(struct nvme_command) != 64); 0add5e8e drivers/nvme/host/pci.c Johannes Thumshirn 2017-06-07 214 BUILD_BUG_ON(sizeof(struct nvme_id_ctrl) != NVME_IDENTIFY_DATA_SIZE); 0add5e8e drivers/nvme/host/pci.c Johannes Thumshirn 2017-06-07 215 BUILD_BUG_ON(sizeof(struct nvme_id_ns) != NVME_IDENTIFY_DATA_SIZE); b60503ba drivers/block/nvme.c Matthew Wilcox 2011-01-20 216 BUILD_BUG_ON(sizeof(struct nvme_lba_range_type) != 64); 6ecec745 drivers/block/nvme.c Keith Busch 2012-09-26 217 BUILD_BUG_ON(sizeof(struct nvme_smart_log) != 512); f9f38e33 drivers/nvme/host/pci.c Helen Koike 2017-04-10 218 BUILD_BUG_ON(sizeof(struct nvme_dbbuf) != 64); f9f38e33 drivers/nvme/host/pci.c Helen Koike 2017-04-10 219 } f9f38e33 drivers/nvme/host/pci.c Helen Koike 2017-04-10 220 :::::: The code at line 206 was first introduced by commit :::::: b60503ba432b16fc84442a84e29a7aad2c0c363d NVMe: New driver :::::: TO: Matthew Wilcox <matthew.r.wilcox@intel.com> :::::: CC: Matthew Wilcox <matthew.r.wilcox@intel.com> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
We don't have a real use-case for this. Understanding the consequences, we are questioning the patch in downstream itself. Please ignore this patch for now. On 12/12/18 1:36 PM, Christoph Hellwig wrote: > scatterlist elements longer than 4GB sound odd. Please submit it > in a series with your actual user so that we can help figuring out > if it really makes sense or if there is a better way to solve your > problem. > > As is this patch will massively increase the memory usage for all users > of struct scatterlist, so you better have a really good reason. >
diff --git a/crypto/shash.c b/crypto/shash.c index d21f04d..434970e 100644 --- a/crypto/shash.c +++ b/crypto/shash.c @@ -298,7 +298,7 @@ int shash_ahash_digest(struct ahash_request *req, struct shash_desc *desc) if (nbytes && (sg = req->src, offset = sg->offset, - nbytes < min(sg->length, ((unsigned int)(PAGE_SIZE)) - offset))) { + nbytes < min(sg->length, ((size_t)(PAGE_SIZE)) - offset))) { void *data; data = kmap_atomic(sg_page(sg)); diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c index c5ea0fc..675def6 100644 --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -810,7 +810,7 @@ static int __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes) offset %= PAGE_SIZE; /* don't overrun current sg */ - count = min(sg->length - qc->cursg_ofs, bytes); + count = min(sg->length - qc->cursg_ofs, (size_t)bytes); /* don't cross page boundaries */ count = min(count, (unsigned int)PAGE_SIZE - offset); diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 99bdae5..bd84c0c 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1025,7 +1025,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd) if (unlikely(length_mask | offset_mask)) { for_each_sg(data->sg, sg, data->sg_len, i) { if (sg->length & length_mask) { - DBG("Reverting to PIO because of transfer size (%d)\n", + DBG("Reverting to PIO because of transfer size (%zd)\n", sg->length); host->flags &= ~SDHCI_REQ_USE_DMA; break; diff --git a/drivers/mmc/host/toshsd.c b/drivers/mmc/host/toshsd.c index dd961c5..af00936 100644 --- a/drivers/mmc/host/toshsd.c +++ b/drivers/mmc/host/toshsd.c @@ -479,7 +479,7 @@ static void toshsd_start_data(struct toshsd_host *host, struct mmc_data *data) { unsigned int flags = SG_MITER_ATOMIC; - dev_dbg(&host->pdev->dev, "setup data transfer: blocksize %08x nr_blocks %d, offset: %08x\n", + dev_dbg(&host->pdev->dev, "setup data transfer: blocksize %08x nr_blocks %d, offset: %08lx\n", data->blksz, data->blocks, data->sg->offset); host->data = data; diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c index cd8b1b9..5fce5ff 100644 --- a/drivers/mmc/host/usdhi6rol0.c +++ b/drivers/mmc/host/usdhi6rol0.c @@ -316,7 +316,7 @@ static void usdhi6_blk_bounce(struct usdhi6_host *host, struct mmc_data *data = host->mrq->data; size_t blk_head = host->head_len; - dev_dbg(mmc_dev(host->mmc), "%s(): CMD%u of %u SG: %ux%u @ 0x%x\n", + dev_dbg(mmc_dev(host->mmc), "%s(): CMD%u of %u SG: %ux%u @ 0x%lx\n", __func__, host->mrq->cmd->opcode, data->sg_len, data->blksz, data->blocks, sg->offset); @@ -360,7 +360,7 @@ static void *usdhi6_sg_map(struct usdhi6_host *host) WARN(host->pg.page, "%p not properly unmapped!\n", host->pg.page); if (WARN(sg_dma_len(sg) % data->blksz, - "SG size %u isn't a multiple of block size %u\n", + "SG size %zu isn't a multiple of block size %u\n", sg_dma_len(sg), data->blksz)) return NULL; @@ -383,7 +383,7 @@ static void *usdhi6_sg_map(struct usdhi6_host *host) else host->blk_page = host->pg.mapped; - dev_dbg(mmc_dev(host->mmc), "Mapped %p (%lx) at %p + %u for CMD%u @ 0x%p\n", + dev_dbg(mmc_dev(host->mmc), "Mapped %p (%lx) at %p + %lu for CMD%u @ 0x%p\n", host->pg.page, page_to_pfn(host->pg.page), host->pg.mapped, sg->offset, host->mrq->cmd->opcode, host->mrq); @@ -492,7 +492,7 @@ static void usdhi6_sg_advance(struct usdhi6_host *host) host->sg = next; if (WARN(next && sg_dma_len(next) % data->blksz, - "SG size %u isn't a multiple of block size %u\n", + "SG size %zu isn't a multiple of block size %u\n", sg_dma_len(next), data->blksz)) data->error = -EINVAL; @@ -1044,7 +1044,7 @@ static int usdhi6_rq_start(struct usdhi6_host *host) (data->blksz % 4 || data->sg->offset % 4)) dev_dbg(mmc_dev(host->mmc), - "Bad SG of %u: %ux%u @ %u\n", data->sg_len, + "Bad SG of %u: %ux%u @ %lu\n", data->sg_len, data->blksz, data->blocks, data->sg->offset); /* Enable DMA for USDHI6_MIN_DMA bytes or more */ @@ -1056,7 +1056,7 @@ static int usdhi6_rq_start(struct usdhi6_host *host) usdhi6_write(host, USDHI6_CC_EXT_MODE, USDHI6_CC_EXT_MODE_SDRW); dev_dbg(mmc_dev(host->mmc), - "%s(): request opcode %u, %u blocks of %u bytes in %u segments, %s %s @+0x%x%s\n", + "%s(): request opcode %u, %u blocks of %u bytes in %u segments, %s %s @+0x%lx%s\n", __func__, cmd->opcode, data->blocks, data->blksz, data->sg_len, use_dma ? "DMA" : "PIO", data->flags & MMC_DATA_READ ? "read" : "write", @@ -1704,7 +1704,7 @@ static void usdhi6_timeout_work(struct work_struct *work) case USDHI6_WAIT_FOR_WRITE: sg = host->sg ?: data->sg; dev_dbg(mmc_dev(host->mmc), - "%c: page #%u @ +0x%zx %ux%u in SG%u. Current SG %u bytes @ %u\n", + "%c: page #%u @ +0x%zx %ux%u in SG%u. Current SG %zu bytes @ %lu\n", data->flags & MMC_DATA_READ ? 'R' : 'W', host->page_idx, host->offset, data->blocks, data->blksz, data->sg_len, sg_dma_len(sg), sg->offset); diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index d668682..57ef89d 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -43,7 +43,7 @@ * require an sg allocation that needs more than a page of data. */ #define NVME_MAX_KB_SZ 4096 -#define NVME_MAX_SEGS 127 +#define NVME_MAX_SEGS 88 static int use_threaded_interrupts; module_param(use_threaded_interrupts, int, 0); @@ -552,8 +552,8 @@ static void nvme_print_sgl(struct scatterlist *sgl, int nents) for_each_sg(sgl, sg, nents, i) { dma_addr_t phys = sg_phys(sg); - pr_warn("sg[%d] phys_addr:%pad offset:%d length:%d " - "dma_address:%pad dma_length:%d\n", + pr_warn("sg[%d] phys_addr:%pad offset:%lu length:%zu " + "dma_address:%pad dma_length:%zu\n", i, &phys, sg->offset, sg->length, &sg_dma_address(sg), sg_dma_len(sg)); } @@ -566,7 +566,7 @@ static blk_status_t nvme_pci_setup_prps(struct nvme_dev *dev, struct dma_pool *pool; int length = blk_rq_payload_bytes(req); struct scatterlist *sg = iod->sg; - int dma_len = sg_dma_len(sg); + u64 dma_len = sg_dma_len(sg); u64 dma_addr = sg_dma_address(sg); u32 page_size = dev->ctrl.page_size; int offset = dma_addr & (page_size - 1); diff --git a/include/linux/nvme.h b/include/linux/nvme.h index 68e91ef..0a07a29 100644 --- a/include/linux/nvme.h +++ b/include/linux/nvme.h @@ -587,7 +587,7 @@ enum { struct nvme_sgl_desc { __le64 addr; - __le32 length; + __le64 length; __u8 rsvd[3]; __u8 type; }; diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 093aa57..f6d3482 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -10,11 +10,11 @@ struct scatterlist { unsigned long page_link; - unsigned int offset; - unsigned int length; + unsigned long offset; + size_t length; dma_addr_t dma_address; #ifdef CONFIG_NEED_SG_DMA_LENGTH - unsigned int dma_length; + size_t dma_length; #endif }; @@ -114,7 +114,7 @@ static inline void sg_assign_page(struct scatterlist *sg, struct page *page) * **/ static inline void sg_set_page(struct scatterlist *sg, struct page *page, - unsigned int len, unsigned int offset) + size_t len, unsigned long offset) { sg_assign_page(sg, page); sg->offset = offset;