Re: [PATCH] Fix writing mtdoops to nand flash.
Hi Bors, thanks for your quick reply. On Mon, Oct 30, 2017 at 3:23 AM, Boris Brezillon <boris.brezil...@free-electrons.com> wrote: > Hi Brent, > > Subject should be prefixed by "mtd: nand: ", so > > "mtd: nand: Fix writing mtdoops to nand flash" Oops, yes, will fix. > > On Sun, 29 Oct 2017 23:23:43 -0500 > moto...@gmail.com wrote: > >> From: Brent Taylor <moto...@gmail.com> >> >> When mtdoops calls mtd_panic_write, it eventually calls >> panic_nand_write in nand_base.c. In order to properly >> wait for the nand chip to be ready in panic_nand_wait, >> the chip must first be selected. >> >> When using the atmel nand flash controller, a panic >> would occur due to a NULL pointer exception. >> >> Signed-off-by: Brent Taylor <moto...@gmail.com> >> --- >> drivers/mtd/nand/nand_base.c | 5 + >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c >> index 12edaae17d81..0a8058a66d93 100644 >> --- a/drivers/mtd/nand/nand_base.c >> +++ b/drivers/mtd/nand/nand_base.c >> @@ -2802,9 +2802,14 @@ static int panic_nand_write(struct mtd_info *mtd, >> loff_t to, size_t len, >> struct mtd_oob_ops ops; >> int ret; >> >> + int chipnr = (int)(to >> chip->chip_shift); >> + chip->select_chip(mtd, chipnr); >> + >> /* Wait for the device to get ready */ >> panic_nand_wait(mtd, chip, 400); >> >> + chip->select_chip(mtd, -1); >> + > > Duh! Looks like a piece of code that is never tested. Did you face the > problem or did you find out by inspecting the code? I was playing with another driver on an atmel development board (at91sam9m10g45ek) and caused a panic with mtdoops enabled. While writing the mtdoops to nand, another panic occurred which caused a storm of panics to generated. > > Anyway, I fear the atmel driver is not the only one to rely on the chip > to be selected when ->dev_ready() is called so this should definitely > be fixed. Also, we should probably move the ->select_chip() and > panic_nand_wait() calls after panic_nand_get_device(), and I don't > think we need to unselect the chip (it will be taken care of by > nand_do_write_ops()). > >> /* Grab the device */ >> panic_nand_get_device(chip, mtd, FL_WRITING); >> > After looking at this closer, a panic could happen at any point correct? If that's the case, the kernel could have been in the middle of a nand read/write operation (which may or may not be on the same chip). Would the chip the mtdoops is being written to need to be reset? I haven't drilled down into the nand_reset function yet, but can that be called in a "panic" state?
Re: [PATCH] Fix writing mtdoops to nand flash.
Hi Bors, thanks for your quick reply. On Mon, Oct 30, 2017 at 3:23 AM, Boris Brezillon wrote: > Hi Brent, > > Subject should be prefixed by "mtd: nand: ", so > > "mtd: nand: Fix writing mtdoops to nand flash" Oops, yes, will fix. > > On Sun, 29 Oct 2017 23:23:43 -0500 > moto...@gmail.com wrote: > >> From: Brent Taylor >> >> When mtdoops calls mtd_panic_write, it eventually calls >> panic_nand_write in nand_base.c. In order to properly >> wait for the nand chip to be ready in panic_nand_wait, >> the chip must first be selected. >> >> When using the atmel nand flash controller, a panic >> would occur due to a NULL pointer exception. >> >> Signed-off-by: Brent Taylor >> --- >> drivers/mtd/nand/nand_base.c | 5 + >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c >> index 12edaae17d81..0a8058a66d93 100644 >> --- a/drivers/mtd/nand/nand_base.c >> +++ b/drivers/mtd/nand/nand_base.c >> @@ -2802,9 +2802,14 @@ static int panic_nand_write(struct mtd_info *mtd, >> loff_t to, size_t len, >> struct mtd_oob_ops ops; >> int ret; >> >> + int chipnr = (int)(to >> chip->chip_shift); >> + chip->select_chip(mtd, chipnr); >> + >> /* Wait for the device to get ready */ >> panic_nand_wait(mtd, chip, 400); >> >> + chip->select_chip(mtd, -1); >> + > > Duh! Looks like a piece of code that is never tested. Did you face the > problem or did you find out by inspecting the code? I was playing with another driver on an atmel development board (at91sam9m10g45ek) and caused a panic with mtdoops enabled. While writing the mtdoops to nand, another panic occurred which caused a storm of panics to generated. > > Anyway, I fear the atmel driver is not the only one to rely on the chip > to be selected when ->dev_ready() is called so this should definitely > be fixed. Also, we should probably move the ->select_chip() and > panic_nand_wait() calls after panic_nand_get_device(), and I don't > think we need to unselect the chip (it will be taken care of by > nand_do_write_ops()). > >> /* Grab the device */ >> panic_nand_get_device(chip, mtd, FL_WRITING); >> > After looking at this closer, a panic could happen at any point correct? If that's the case, the kernel could have been in the middle of a nand read/write operation (which may or may not be on the same chip). Would the chip the mtdoops is being written to need to be reset? I haven't drilled down into the nand_reset function yet, but can that be called in a "panic" state?
Build error in timer-atmel-pit.c
Daniel, After updating to linux-4.8-rc4, I got the following build error: linux-x.yy/drivers/clocksource/timer-atmel-pit.c: In function 'at91sam926x_pit_dt_init': linux-x.yy/drivers/clocksource/timer-atmel-pit.c:264:2: error: 'ret' undeclared (first use in this function) ret = clk_prepare_enable(data->mck); ^~~ linux-x.yy/drivers/clocksource/timer-atmel-pit.c:264:2: note: each undeclared identifier is reported only once for each function it appears in This was introduced in commit: 699e36e5b8e9f77b2be4c23f0b309e53be4b2880 Regards, Brent Taylor
Build error in timer-atmel-pit.c
Daniel, After updating to linux-4.8-rc4, I got the following build error: linux-x.yy/drivers/clocksource/timer-atmel-pit.c: In function 'at91sam926x_pit_dt_init': linux-x.yy/drivers/clocksource/timer-atmel-pit.c:264:2: error: 'ret' undeclared (first use in this function) ret = clk_prepare_enable(data->mck); ^~~ linux-x.yy/drivers/clocksource/timer-atmel-pit.c:264:2: note: each undeclared identifier is reported only once for each function it appears in This was introduced in commit: 699e36e5b8e9f77b2be4c23f0b309e53be4b2880 Regards, Brent Taylor
at_xdmac: txd used outside spinlock after being released?
Hi Ludovic,
I'm learning about the dmaengine subsystem and I was using the
at_xdmac as a reference.I'm not real familiar with tasklets
because I have used threaded interrupt handlers instead of them.
I noticed that the variable "txd" in the following block of code
(from the function at_xdmac_tasklet) is used after releasing the
atchan->lock and after the "desc" the txd is associated with is
returned back to the free descriptor list.
spin_lock_bh(>lock);
desc = list_first_entry(>xfers_list,
struct at_xdmac_desc,
xfer_node);
...
txd = >tx_dma_desc;
at_xdmac_remove_xfer(atchan, desc);
spin_unlock_bh(>lock);
if (!at_xdmac_chan_is_cyclic(atchan)) {
dma_cookie_complete(txd);
if (txd->callback && (txd->flags & DMA_PREP_INTERRUPT))
txd->callback(txd->callback_param);
}
dma_run_dependencies(txd);
Is there any danger of another process (maybe running on another
processor) changing desc->txd after it has been put back on the
free_list?
My first thought was to move the spin_unlock_bh(>lock) untill
after dma_run_dependencies(txd), but a deadlock will be introduced
because dma_run_dependencies could invoke at_xdmac_issue_pending which
could eventually call spin_lock_irqsave(>lock). A deadlock
could also be created if the "callback" function invoked another
"device_prep_*" function.
If I'm miss-understanding something, I apologize for the noise.
-- Brent Taylor
at_xdmac: txd used outside spinlock after being released?
Hi Ludovic,
I'm learning about the dmaengine subsystem and I was using the
at_xdmac as a reference.I'm not real familiar with tasklets
because I have used threaded interrupt handlers instead of them.
I noticed that the variable "txd" in the following block of code
(from the function at_xdmac_tasklet) is used after releasing the
atchan->lock and after the "desc" the txd is associated with is
returned back to the free descriptor list.
spin_lock_bh(>lock);
desc = list_first_entry(>xfers_list,
struct at_xdmac_desc,
xfer_node);
...
txd = >tx_dma_desc;
at_xdmac_remove_xfer(atchan, desc);
spin_unlock_bh(>lock);
if (!at_xdmac_chan_is_cyclic(atchan)) {
dma_cookie_complete(txd);
if (txd->callback && (txd->flags & DMA_PREP_INTERRUPT))
txd->callback(txd->callback_param);
}
dma_run_dependencies(txd);
Is there any danger of another process (maybe running on another
processor) changing desc->txd after it has been put back on the
free_list?
My first thought was to move the spin_unlock_bh(>lock) untill
after dma_run_dependencies(txd), but a deadlock will be introduced
because dma_run_dependencies could invoke at_xdmac_issue_pending which
could eventually call spin_lock_irqsave(>lock). A deadlock
could also be created if the "callback" function invoked another
"device_prep_*" function.
If I'm miss-understanding something, I apologize for the noise.
-- Brent Taylor
[PATCH] mmc: atmel-mci: Check pdata for NULL before dereferencing it
I'm using an at91sam9g20ek development board, and I ran into a kernel panic with 4.5.0-rc7: atmel_mci fffa8000.mmc: version: 0x210 Unable to handle kernel NULL pointer dereference at virtual address 0004 pgd = c0004000 [0004] *pgd= Internal error: Oops: 5 [#1] ARM Modules linked in: CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc7 #5 Hardware name: Atmel AT91SAM9 task: c3838000 ti: c383c000 task.ti: c383c000 PC is at atmci_probe+0x3f8/0x7c4 LR is at dma_request_chan+0x134/0x158 pc : []lr : []psr: 6013 sp : c383de40 ip : 6013 fp : 0022 r10: c3910b60 r9 : c38da900 r8 : 0210 r7 : c38da910 r6 : c38c14d0 r5 : r4 : c398b110 r3 : r2 : r1 : 0001 r0 : ffed Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 0005317f Table: 20004000 DAC: 0053 Process swapper (pid: 1, stack limit = 0xc383c190) Stack: (0xc383de40 to 0xc383e000) de40: c38db360 c398b110 c38dc5f0 c0404386 c398dfa0 c0486838 c38da910 de60: c04ae2d0 c04ce424 c0486838 c04bbec0 c01e2318 de80: c38da910 c04ae2d0 c04ce424 c01e0c54 c04ae2d0 c38da910 c38da910 c38da944 dea0: c04ae2d0 c04a6568 c04b81c8 c01e0ee8 c04ae2d0 c01e0e80 c01df188 dec0: c382d58c c38d8570 c04ae2d0 c398c180 c01e0194 c0418298 c041829b dee0: c04ae2d0 c047b1f4 c0497ea0 c0497ea0 c01e147c c39108a0 c047b1f4 df00: c00096c4 c382cb40 c387a280 c387a280 c0332170 c3ffcb00 c3ffcb83 df20: c044baac c002a2a4 c03e5ae4 c382cb40 c382cb40 c044b2dc 0065 0006 df40: 0006 c044bac0 0064 c044bac0 0006 c048682c 0006 df60: c0486830 0065 c0492978 c04bbec0 c0486838 c0468d20 0006 0006 df80: c046859c c032c81c dfa0: c032c824 c000a290 dfc0: dfe0: 0013 [] (atmci_probe) from [] (platform_drv_probe+0x38/0x6c) [] (platform_drv_probe) from [] (driver_probe_device+0x1ac/0x3d8) [] (driver_probe_device) from [] (__driver_attach+0x68/0x8c) [] (__driver_attach) from [] (bus_for_each_dev+0x68/0x8c) [] (bus_for_each_dev) from [] (bus_add_driver+0x110/0x23c) [] (bus_add_driver) from [] (driver_register+0x9c/0xe0) [] (driver_register) from [] (do_one_initcall+0x118/0x1dc) [] (do_one_initcall) from [] (kernel_init_freeable+0xfc/0x1c0) [] (kernel_init_freeable) from [] (kernel_init+0x8/0xe4) [] (kernel_init) from [] (ret_from_fork+0x14/0x24) Code: e5840030 1a14 e59430c8 e5933058 (e5932004) ---[ end trace 607b62d4422f7087 ]--- This occurs because the "host->pdev->dev.patform_data" is NULL because I'm using a device tree to setup all the devices. This patch checks pdata before dereferencing it. Signed-off-by: Brent Taylor gmail.com> --- a/drivers/mmc/host/atmel-mci.c2016-03-13 00:10:57.527773324 -0600 +++ b/drivers/mmc/host/atmel-mci.c2016-03-13 00:10:44.903433138 -0600 @@ -2443,7 +2443,7 @@ static int atmci_configure_dma(struct at struct mci_platform_data *pdata = host->pdev->dev.platform_data; dma_cap_mask_t mask; - if (!pdata->dma_filter) + if (!pdata || !pdata->dma_filter) return -ENODEV; dma_cap_zero(mask);
[PATCH] mmc: atmel-mci: Check pdata for NULL before dereferencing it
I'm using an at91sam9g20ek development board, and I ran into a kernel panic with 4.5.0-rc7: atmel_mci fffa8000.mmc: version: 0x210 Unable to handle kernel NULL pointer dereference at virtual address 0004 pgd = c0004000 [0004] *pgd= Internal error: Oops: 5 [#1] ARM Modules linked in: CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc7 #5 Hardware name: Atmel AT91SAM9 task: c3838000 ti: c383c000 task.ti: c383c000 PC is at atmci_probe+0x3f8/0x7c4 LR is at dma_request_chan+0x134/0x158 pc : []lr : []psr: 6013 sp : c383de40 ip : 6013 fp : 0022 r10: c3910b60 r9 : c38da900 r8 : 0210 r7 : c38da910 r6 : c38c14d0 r5 : r4 : c398b110 r3 : r2 : r1 : 0001 r0 : ffed Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 0005317f Table: 20004000 DAC: 0053 Process swapper (pid: 1, stack limit = 0xc383c190) Stack: (0xc383de40 to 0xc383e000) de40: c38db360 c398b110 c38dc5f0 c0404386 c398dfa0 c0486838 c38da910 de60: c04ae2d0 c04ce424 c0486838 c04bbec0 c01e2318 de80: c38da910 c04ae2d0 c04ce424 c01e0c54 c04ae2d0 c38da910 c38da910 c38da944 dea0: c04ae2d0 c04a6568 c04b81c8 c01e0ee8 c04ae2d0 c01e0e80 c01df188 dec0: c382d58c c38d8570 c04ae2d0 c398c180 c01e0194 c0418298 c041829b dee0: c04ae2d0 c047b1f4 c0497ea0 c0497ea0 c01e147c c39108a0 c047b1f4 df00: c00096c4 c382cb40 c387a280 c387a280 c0332170 c3ffcb00 c3ffcb83 df20: c044baac c002a2a4 c03e5ae4 c382cb40 c382cb40 c044b2dc 0065 0006 df40: 0006 c044bac0 0064 c044bac0 0006 c048682c 0006 df60: c0486830 0065 c0492978 c04bbec0 c0486838 c0468d20 0006 0006 df80: c046859c c032c81c dfa0: c032c824 c000a290 dfc0: dfe0: 0013 [] (atmci_probe) from [] (platform_drv_probe+0x38/0x6c) [] (platform_drv_probe) from [] (driver_probe_device+0x1ac/0x3d8) [] (driver_probe_device) from [] (__driver_attach+0x68/0x8c) [] (__driver_attach) from [] (bus_for_each_dev+0x68/0x8c) [] (bus_for_each_dev) from [] (bus_add_driver+0x110/0x23c) [] (bus_add_driver) from [] (driver_register+0x9c/0xe0) [] (driver_register) from [] (do_one_initcall+0x118/0x1dc) [] (do_one_initcall) from [] (kernel_init_freeable+0xfc/0x1c0) [] (kernel_init_freeable) from [] (kernel_init+0x8/0xe4) [] (kernel_init) from [] (ret_from_fork+0x14/0x24) Code: e5840030 1a14 e59430c8 e5933058 (e5932004) ---[ end trace 607b62d4422f7087 ]--- This occurs because the "host->pdev->dev.patform_data" is NULL because I'm using a device tree to setup all the devices. This patch checks pdata before dereferencing it. Signed-off-by: Brent Taylor gmail.com> --- a/drivers/mmc/host/atmel-mci.c2016-03-13 00:10:57.527773324 -0600 +++ b/drivers/mmc/host/atmel-mci.c2016-03-13 00:10:44.903433138 -0600 @@ -2443,7 +2443,7 @@ static int atmci_configure_dma(struct at struct mci_platform_data *pdata = host->pdev->dev.platform_data; dma_cap_mask_t mask; - if (!pdata->dma_filter) + if (!pdata || !pdata->dma_filter) return -ENODEV; dma_cap_zero(mask);
Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
On Mon, Dec 21, 2015 at 1:23 PM, Souptick Joarder wrote: > Hi Brent, > > On Tue, Dec 1, 2015 at 11:11 AM, Brent Taylor wrote: >> Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to >> be pointing to memory allocated by vmalloc. If the api1 method (via >> ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is >> used. This patch checks if the firmware being loaded is the 'fw' image, >> then use vmalloc, otherwise use kmalloc. >> >> Signed-off-by: Brent Taylor >> --- >> drivers/net/wireless/ath/ath6kl/init.c | 7 ++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/ath/ath6kl/init.c >> b/drivers/net/wireless/ath/ath6kl/init.c >> index 6ae0734..4f2b124d 100644 >> --- a/drivers/net/wireless/ath/ath6kl/init.c >> +++ b/drivers/net/wireless/ath/ath6kl/init.c >> @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char >> *filename, >> return ret; >> >> *fw_len = fw_entry->size; >> - *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL); >> + if (>fw == fw) >> + *fw = vmalloc(fw_entry->size); >> + else >> + *fw = kmalloc(fw_entry->size, GFP_KERNEL); > > Why vmalloc and kmalloc both are required? can't use either > vmalloc or kmalloc? My original problem was that kmemdup (which uses kmalloc) could not allocate enough memory to hold the firmware that is placed into "ar->fw". In the function ath6kl_core_cleanup (in core.c), the "ar->fw" pointer is the only one that uses vfree which was changed in commit 8437754c83351d6213c1a47ff029c1126d6042a7. I was trying to change as little as possible and I wasn't sure if there was a reason that any of the other firmware items needed to be allocated with kmalloc or if they could be changed to use vmalloc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
On Mon, Dec 21, 2015 at 1:23 PM, Souptick Joarder <jrdr.li...@gmail.com> wrote: > Hi Brent, > > On Tue, Dec 1, 2015 at 11:11 AM, Brent Taylor <moto...@gmail.com> wrote: >> Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to >> be pointing to memory allocated by vmalloc. If the api1 method (via >> ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is >> used. This patch checks if the firmware being loaded is the 'fw' image, >> then use vmalloc, otherwise use kmalloc. >> >> Signed-off-by: Brent Taylor <moto...@gmail.com> >> --- >> drivers/net/wireless/ath/ath6kl/init.c | 7 ++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/ath/ath6kl/init.c >> b/drivers/net/wireless/ath/ath6kl/init.c >> index 6ae0734..4f2b124d 100644 >> --- a/drivers/net/wireless/ath/ath6kl/init.c >> +++ b/drivers/net/wireless/ath/ath6kl/init.c >> @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char >> *filename, >> return ret; >> >> *fw_len = fw_entry->size; >> - *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL); >> + if (>fw == fw) >> + *fw = vmalloc(fw_entry->size); >> + else >> + *fw = kmalloc(fw_entry->size, GFP_KERNEL); > > Why vmalloc and kmalloc both are required? can't use either > vmalloc or kmalloc? My original problem was that kmemdup (which uses kmalloc) could not allocate enough memory to hold the firmware that is placed into "ar->fw". In the function ath6kl_core_cleanup (in core.c), the "ar->fw" pointer is the only one that uses vfree which was changed in commit 8437754c83351d6213c1a47ff029c1126d6042a7. I was trying to change as little as possible and I wasn't sure if there was a reason that any of the other firmware items needed to be allocated with kmalloc or if they could be changed to use vmalloc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[v2] ath6kl: Use vmalloc to allocate ar->fw for api1 method
Since commit 8437754c8335 ("ath6kl: Use vmalloc instead of kmalloc for
fw") ar->fw is expected to be pointing to memory allocated by vmalloc.
If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory
for ar->fw, then kmemdup is used. This patch checks if the firmware being
loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.
Signed-off-by: Brent Taylor
---
v2: Fix commit message and code formatting (use tab instaed of spaces)
drivers/net/wireless/ath/ath6kl/init.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath6kl/init.c
b/drivers/net/wireless/ath/ath6kl/init.c
index 6ae0734..4f16bd8 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char
*filename,
return ret;
*fw_len = fw_entry->size;
- *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
+ if (>fw == fw)
+ *fw = vmalloc(fw_entry->size);
+ else
+ *fw = kmalloc(fw_entry->size, GFP_KERNEL);
if (*fw == NULL)
ret = -ENOMEM;
+ else
+ memcpy(*fw, fw_entry->data, fw_entry->size);
release_firmware(fw_entry);
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[v2] ath6kl: Use vmalloc to allocate ar->fw for api1 method
Since commit 8437754c8335 ("ath6kl: Use vmalloc instead of kmalloc for
fw") ar->fw is expected to be pointing to memory allocated by vmalloc.
If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory
for ar->fw, then kmemdup is used. This patch checks if the firmware being
loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc.
Signed-off-by: Brent Taylor <moto...@gmail.com>
---
v2: Fix commit message and code formatting (use tab instaed of spaces)
drivers/net/wireless/ath/ath6kl/init.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath6kl/init.c
b/drivers/net/wireless/ath/ath6kl/init.c
index 6ae0734..4f16bd8 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char
*filename,
return ret;
*fw_len = fw_entry->size;
- *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
+ if (>fw == fw)
+ *fw = vmalloc(fw_entry->size);
+ else
+ *fw = kmalloc(fw_entry->size, GFP_KERNEL);
if (*fw == NULL)
ret = -ENOMEM;
+ else
+ memcpy(*fw, fw_entry->data, fw_entry->size);
release_firmware(fw_entry);
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to be pointing to memory allocated by vmalloc. If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used. This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc. Signed-off-by: Brent Taylor --- drivers/net/wireless/ath/ath6kl/init.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c index 6ae0734..4f2b124d 100644 --- a/drivers/net/wireless/ath/ath6kl/init.c +++ b/drivers/net/wireless/ath/ath6kl/init.c @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename, return ret; *fw_len = fw_entry->size; - *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL); + if (>fw == fw) + *fw = vmalloc(fw_entry->size); + else + *fw = kmalloc(fw_entry->size, GFP_KERNEL); if (*fw == NULL) ret = -ENOMEM; + else + memcpy(*fw, fw_entry->data, fw_entry->size); release_firmware(fw_entry); -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ath6kl: Use vmalloc to allocate ar->fw for api1 method
Since commit 8437754c83351d6213c1a47ff029c1126d6042a7, ar->fw is expected to be pointing to memory allocated by vmalloc. If the api1 method (via ath6kl_fetch_fw_api1) is used to allocate memory for ar->fw, then kmemdup is used. This patch checks if the firmware being loaded is the 'fw' image, then use vmalloc, otherwise use kmalloc. Signed-off-by: Brent Taylor <moto...@gmail.com> --- drivers/net/wireless/ath/ath6kl/init.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c index 6ae0734..4f2b124d 100644 --- a/drivers/net/wireless/ath/ath6kl/init.c +++ b/drivers/net/wireless/ath/ath6kl/init.c @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename, return ret; *fw_len = fw_entry->size; - *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL); + if (>fw == fw) + *fw = vmalloc(fw_entry->size); + else + *fw = kmalloc(fw_entry->size, GFP_KERNEL); if (*fw == NULL) ret = -ENOMEM; + else + memcpy(*fw, fw_entry->data, fw_entry->size); release_firmware(fw_entry); -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3] ath6kl: Use vmalloc for loading firmware using api1 method and use kvfree
Signed-off-by: Brent Taylor ath6kl: Use vmalloc for loading firmware using api1 method and free using kvfree ath6kl: fix kmalloc build error --- Changes v2 -> v3: - fix kmalloc build error Changes v1 -> v2: - simplify memory allocation - use kvfree drivers/net/wireless/ath/ath6kl/core.c | 2 +- drivers/net/wireless/ath/ath6kl/init.c | 7 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath6kl/core.c b/drivers/net/wireless/ath/ath6kl/core.c index 4ec02ce..052e58b 100644 --- a/drivers/net/wireless/ath/ath6kl/core.c +++ b/drivers/net/wireless/ath/ath6kl/core.c @@ -343,7 +343,7 @@ void ath6kl_core_cleanup(struct ath6kl *ar) kfree(ar->fw_board); kfree(ar->fw_otp); - vfree(ar->fw); + kvfree(ar->fw); kfree(ar->fw_patch); kfree(ar->fw_testscript); diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c index 6e473fa..19535dc 100644 --- a/drivers/net/wireless/ath/ath6kl/init.c +++ b/drivers/net/wireless/ath/ath6kl/init.c @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename, return ret; *fw_len = fw_entry->size; - *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL); + if (>fw == fw) + *fw = vmalloc(fw_entry->size); + else + *fw = kmalloc(fw_entry->size, GFP_KERNEL); if (*fw == NULL) ret = -ENOMEM; + else + memcpy(*fw, fw_entry->data, fw_entry->size); release_firmware(fw_entry); -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] ath6kl: Use vmalloc for loading firmware using api1 method and use kvfree
Signed-off-by: Brent Taylor ath6kl: Use vmalloc for loading firmware using api1 method and free using kvfree --- Changes v1 -> v2: - simplify memory allocation - use kvfree drivers/net/wireless/ath/ath6kl/core.c | 2 +- drivers/net/wireless/ath/ath6kl/init.c | 7 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath6kl/core.c b/drivers/net/wireless/ath/ath6kl/core.c index 4ec02ce..052e58b 100644 --- a/drivers/net/wireless/ath/ath6kl/core.c +++ b/drivers/net/wireless/ath/ath6kl/core.c @@ -343,7 +343,7 @@ void ath6kl_core_cleanup(struct ath6kl *ar) kfree(ar->fw_board); kfree(ar->fw_otp); - vfree(ar->fw); + kvfree(ar->fw); kfree(ar->fw_patch); kfree(ar->fw_testscript); diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c index 6e473fa..836afea2 100644 --- a/drivers/net/wireless/ath/ath6kl/init.c +++ b/drivers/net/wireless/ath/ath6kl/init.c @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename, return ret; *fw_len = fw_entry->size; - *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL); + if (>fw == fw) + *fw = vmalloc(fw_entry->size); + else + *fw = kmalloc(fw_entry->size); if (*fw == NULL) ret = -ENOMEM; + else + memcpy(*fw, fw_entry->data, fw_entry->size); release_firmware(fw_entry); -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ath6kl: Use vmalloc for loading firmware using api1 method
Sorry, the first e-mail was sent via gmail and I forgot about sending
it in plain text mode.
Whats the status on this patch? I don't see it on patchwork anymore
nor is it in any of the git trees I checked.
Thanks,
Brent
On Fri, Oct 16, 2015 at 12:10 AM, Brent Taylor wrote:
> Signed-off-by: Brent Taylor
> ---
> drivers/net/wireless/ath/ath6kl/init.c | 15 +++
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath6kl/init.c
> b/drivers/net/wireless/ath/ath6kl/init.c
> index 6e473fa..2155739 100644
> --- a/drivers/net/wireless/ath/ath6kl/init.c
> +++ b/drivers/net/wireless/ath/ath6kl/init.c
> @@ -673,10 +673,17 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char
> *filename,
> return ret;
>
> *fw_len = fw_entry->size;
> - *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
> -
> - if (*fw == NULL)
> - ret = -ENOMEM;
> + if (>fw == fw) {
> + *fw = vmalloc(fw_entry->size);
> + if (*fw == NULL)
> + ret = -ENOMEM;
> + else
> + memcpy(*fw, fw_entry->data, fw_entry->size);
> + } else {
> + *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
> + if (*fw == NULL)
> + ret = -ENOMEM;
> + }
>
> release_firmware(fw_entry);
>
> --
> 2.6.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ath6kl: Use vmalloc for loading firmware using api1 method
Sorry, the first e-mail was sent via gmail and I forgot about sending
it in plain text mode.
Whats the status on this patch? I don't see it on patchwork anymore
nor is it in any of the git trees I checked.
Thanks,
Brent
On Fri, Oct 16, 2015 at 12:10 AM, Brent Taylor <moto...@gmail.com> wrote:
> Signed-off-by: Brent Taylor <moto...@gmail.com>
> ---
> drivers/net/wireless/ath/ath6kl/init.c | 15 +++
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath6kl/init.c
> b/drivers/net/wireless/ath/ath6kl/init.c
> index 6e473fa..2155739 100644
> --- a/drivers/net/wireless/ath/ath6kl/init.c
> +++ b/drivers/net/wireless/ath/ath6kl/init.c
> @@ -673,10 +673,17 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char
> *filename,
> return ret;
>
> *fw_len = fw_entry->size;
> - *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
> -
> - if (*fw == NULL)
> - ret = -ENOMEM;
> + if (>fw == fw) {
> + *fw = vmalloc(fw_entry->size);
> + if (*fw == NULL)
> + ret = -ENOMEM;
> + else
> + memcpy(*fw, fw_entry->data, fw_entry->size);
> + } else {
> + *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
> + if (*fw == NULL)
> + ret = -ENOMEM;
> + }
>
> release_firmware(fw_entry);
>
> --
> 2.6.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] ath6kl: Use vmalloc for loading firmware using api1 method and use kvfree
Signed-off-by: Brent Taylor <moto...@gmail.com> ath6kl: Use vmalloc for loading firmware using api1 method and free using kvfree --- Changes v1 -> v2: - simplify memory allocation - use kvfree drivers/net/wireless/ath/ath6kl/core.c | 2 +- drivers/net/wireless/ath/ath6kl/init.c | 7 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath6kl/core.c b/drivers/net/wireless/ath/ath6kl/core.c index 4ec02ce..052e58b 100644 --- a/drivers/net/wireless/ath/ath6kl/core.c +++ b/drivers/net/wireless/ath/ath6kl/core.c @@ -343,7 +343,7 @@ void ath6kl_core_cleanup(struct ath6kl *ar) kfree(ar->fw_board); kfree(ar->fw_otp); - vfree(ar->fw); + kvfree(ar->fw); kfree(ar->fw_patch); kfree(ar->fw_testscript); diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c index 6e473fa..836afea2 100644 --- a/drivers/net/wireless/ath/ath6kl/init.c +++ b/drivers/net/wireless/ath/ath6kl/init.c @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename, return ret; *fw_len = fw_entry->size; - *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL); + if (>fw == fw) + *fw = vmalloc(fw_entry->size); + else + *fw = kmalloc(fw_entry->size); if (*fw == NULL) ret = -ENOMEM; + else + memcpy(*fw, fw_entry->data, fw_entry->size); release_firmware(fw_entry); -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3] ath6kl: Use vmalloc for loading firmware using api1 method and use kvfree
Signed-off-by: Brent Taylor <moto...@gmail.com> ath6kl: Use vmalloc for loading firmware using api1 method and free using kvfree ath6kl: fix kmalloc build error --- Changes v2 -> v3: - fix kmalloc build error Changes v1 -> v2: - simplify memory allocation - use kvfree drivers/net/wireless/ath/ath6kl/core.c | 2 +- drivers/net/wireless/ath/ath6kl/init.c | 7 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath6kl/core.c b/drivers/net/wireless/ath/ath6kl/core.c index 4ec02ce..052e58b 100644 --- a/drivers/net/wireless/ath/ath6kl/core.c +++ b/drivers/net/wireless/ath/ath6kl/core.c @@ -343,7 +343,7 @@ void ath6kl_core_cleanup(struct ath6kl *ar) kfree(ar->fw_board); kfree(ar->fw_otp); - vfree(ar->fw); + kvfree(ar->fw); kfree(ar->fw_patch); kfree(ar->fw_testscript); diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c index 6e473fa..19535dc 100644 --- a/drivers/net/wireless/ath/ath6kl/init.c +++ b/drivers/net/wireless/ath/ath6kl/init.c @@ -673,10 +673,15 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char *filename, return ret; *fw_len = fw_entry->size; - *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL); + if (>fw == fw) + *fw = vmalloc(fw_entry->size); + else + *fw = kmalloc(fw_entry->size, GFP_KERNEL); if (*fw == NULL) ret = -ENOMEM; + else + memcpy(*fw, fw_entry->data, fw_entry->size); release_firmware(fw_entry); -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ath6kl: Use vmalloc for loading firmware using api1 method
Signed-off-by: Brent Taylor
---
drivers/net/wireless/ath/ath6kl/init.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/init.c
b/drivers/net/wireless/ath/ath6kl/init.c
index 6e473fa..2155739 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -673,10 +673,17 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char
*filename,
return ret;
*fw_len = fw_entry->size;
- *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
-
- if (*fw == NULL)
- ret = -ENOMEM;
+ if (>fw == fw) {
+ *fw = vmalloc(fw_entry->size);
+ if (*fw == NULL)
+ ret = -ENOMEM;
+ else
+ memcpy(*fw, fw_entry->data, fw_entry->size);
+ } else {
+ *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
+ if (*fw == NULL)
+ ret = -ENOMEM;
+ }
release_firmware(fw_entry);
--
2.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ath6kl: Use vmalloc for loading firmware using api1 method
Signed-off-by: Brent Taylor <moto...@gmail.com>
---
drivers/net/wireless/ath/ath6kl/init.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/init.c
b/drivers/net/wireless/ath/ath6kl/init.c
index 6e473fa..2155739 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -673,10 +673,17 @@ static int ath6kl_get_fw(struct ath6kl *ar, const char
*filename,
return ret;
*fw_len = fw_entry->size;
- *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
-
- if (*fw == NULL)
- ret = -ENOMEM;
+ if (>fw == fw) {
+ *fw = vmalloc(fw_entry->size);
+ if (*fw == NULL)
+ ret = -ENOMEM;
+ else
+ memcpy(*fw, fw_entry->data, fw_entry->size);
+ } else {
+ *fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
+ if (*fw == NULL)
+ ret = -ENOMEM;
+ }
release_firmware(fw_entry);
--
2.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: lz4hc compression in UBIFS?
On Wed, Oct 23, 2013 at 2:40 AM, Konstantin Tokarev wrote: > > > 23.10.2013, 09:26, "Brent Taylor" : >> Konstantin, >>I did my testing with data from /dev/urandom (which I now realize >> wasn't the best choice of data source), but if I use /dev/zero (which >> actually causes data compression to occur), the decompressor fails. I >> don't know the internal workings of the lz4hc compressor or the lz4 >> decompressor. I couldn't find any examples of any code in the kernel >> actually using the compressor. I've cc'ed the maintainers of the >> lz4hc_compress.c to see if they my have some more insight to the >> issue. > > Does decompressor fail for you with the same error messages? > > Have you tried to copy my file to the volume? It looks like minimal test case > for my board, if I remove any line decompressor works fine. > > -- > Regards, > Konstantin Yes, I get the same error, here's a dump from UBIFS when I cat a file filled with data from /dev/zero: UBIFS error (pid 4288): ubifs_decompress: cannot decompress 12 bytes, compressor lz4hc, error -22 UBIFS error (pid 4288): read_block: bad data node (block 0, inode 71) magic 0x6101831 crc0xff61a078 node_type 1 (data node) group_type 0 (no node group) sqnum 2700 len60 key(71, data, 0) size 512 compr_typ 3 data size 12 data: : 1f 00 01 00 ff e8 50 00 00 00 00 00 UBIFS error (pid 4288): do_readpage: cannot read page 0 of inode 71, error -22 cat: /opt/data/zero.bin: Input/output error Steps to reproduce are: 1. Create a file with all zeros: dd if=/dev/zero bs=512 count=1 of=/opt/data/zero.bin 2. Unmount ubifs and detach ubi partition 3. attach ubi partition and mount ubifs 4. cat /opt/data/zero.bin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: lz4hc compression in UBIFS?
On Wed, Oct 23, 2013 at 2:40 AM, Konstantin Tokarev annu...@yandex.ru wrote: 23.10.2013, 09:26, Brent Taylor moto...@gmail.com: Konstantin, I did my testing with data from /dev/urandom (which I now realize wasn't the best choice of data source), but if I use /dev/zero (which actually causes data compression to occur), the decompressor fails. I don't know the internal workings of the lz4hc compressor or the lz4 decompressor. I couldn't find any examples of any code in the kernel actually using the compressor. I've cc'ed the maintainers of the lz4hc_compress.c to see if they my have some more insight to the issue. Does decompressor fail for you with the same error messages? Have you tried to copy my file to the volume? It looks like minimal test case for my board, if I remove any line decompressor works fine. -- Regards, Konstantin Yes, I get the same error, here's a dump from UBIFS when I cat a file filled with data from /dev/zero: UBIFS error (pid 4288): ubifs_decompress: cannot decompress 12 bytes, compressor lz4hc, error -22 UBIFS error (pid 4288): read_block: bad data node (block 0, inode 71) magic 0x6101831 crc0xff61a078 node_type 1 (data node) group_type 0 (no node group) sqnum 2700 len60 key(71, data, 0) size 512 compr_typ 3 data size 12 data: : 1f 00 01 00 ff e8 50 00 00 00 00 00 UBIFS error (pid 4288): do_readpage: cannot read page 0 of inode 71, error -22 cat: /opt/data/zero.bin: Input/output error Steps to reproduce are: 1. Create a file with all zeros: dd if=/dev/zero bs=512 count=1 of=/opt/data/zero.bin 2. Unmount ubifs and detach ubi partition 3. attach ubi partition and mount ubifs 4. cat /opt/data/zero.bin -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: lz4hc compression in UBIFS?
Konstantin, I did my testing with data from /dev/urandom (which I now realize wasn't the best choice of data source), but if I use /dev/zero (which actually causes data compression to occur), the decompressor fails. I don't know the internal workings of the lz4hc compressor or the lz4 decompressor. I couldn't find any examples of any code in the kernel actually using the compressor. I've cc'ed the maintainers of the lz4hc_compress.c to see if they my have some more insight to the issue. -- Brent On Tue, Oct 22, 2013 at 5:10 AM, Konstantin Tokarev wrote: > > > 22.10.2013, 07:43, "Brent Taylor" : >> On Mon, Oct 21, 2013 at 10:59 AM, Konstantin Tokarev >> wrote: >> >>> 04.10.2013, 07:09, "Brent Taylor" : >>>> Here is a patch based on linux-3.12-rc3. I haven't performed any >>>> performance testing UBIFS using lz4hc, but I can mount UBIFS volumes >>>> and haven't seen any problems yet. The only think I know that isn't >>>> correct about the patch is the description for the Kconfig element for >>>> select lz4hc as a compression option. I only copied the description >>>> from the lzo description. >>> Hi Brent, >>> >>> I'm testing your patch on my SH4 device. When I create new partition >>> with lz4hc compressor, it works fine: I can copy file into it, and >>> md5sums of original and copy match. However, after reboot I cannot >>> read the file anymore: >>> >>> UBIFS error (pid 1101): ubifs_decompress: cannot decompress 934 bytes, >>> compressor lz4hc, error -22 >>> UBIFS error (pid 1101): read_block: bad data node (block 1, inode 65) >>> UBIFS error (pid 1101): do_readpage: cannot read page 1 of inode 65, error >>> -22 >>> >>> The same error appears if I use lz4hc-compressed ubifs image to flash >>> rootfs >>> (using patched mkfs.ubifs). >>> >>> Decompression error occurs in lz4_uncompress() function >>> (lib/lz4/lz4_decompress.c), >>> on the line 101: >>> >>> /* Error: offset create reference outside destination buffer */ >>> if (unlikely(ref < (BYTE *const) dest)) >>> goto _output_error; >>> >>> Brent: are you able to read data from lz4hc volume on your device? >>> Anyone: any ideas what may happen here? >>> >>> -- >>> Regards, >>> Konstantin >> >> Konstantin, >>I haven't seen anything like that on my at91sam9m10g45-ek >> development board. I haven't used a flash image from mkfs.ubifs yet. >> Is it possible the file system was not umounted cleanly before the >> reboot and UBIFS went through a recovery procedure? Maybe something >> breaks with lz4hc when UBIFS does a recovery? That's just a guess. > > Could you save attached file on lz4hc volume, umount it and mount again? > I get aforementioned error when doing `cat set11.cfg` > > -- > Regards, > Konstantin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: lz4hc compression in UBIFS?
Konstantin, I did my testing with data from /dev/urandom (which I now realize wasn't the best choice of data source), but if I use /dev/zero (which actually causes data compression to occur), the decompressor fails. I don't know the internal workings of the lz4hc compressor or the lz4 decompressor. I couldn't find any examples of any code in the kernel actually using the compressor. I've cc'ed the maintainers of the lz4hc_compress.c to see if they my have some more insight to the issue. -- Brent On Tue, Oct 22, 2013 at 5:10 AM, Konstantin Tokarev annu...@yandex.ru wrote: 22.10.2013, 07:43, Brent Taylor moto...@gmail.com: On Mon, Oct 21, 2013 at 10:59 AM, Konstantin Tokarev annu...@yandex.ru wrote: 04.10.2013, 07:09, Brent Taylor moto...@gmail.com: Here is a patch based on linux-3.12-rc3. I haven't performed any performance testing UBIFS using lz4hc, but I can mount UBIFS volumes and haven't seen any problems yet. The only think I know that isn't correct about the patch is the description for the Kconfig element for select lz4hc as a compression option. I only copied the description from the lzo description. Hi Brent, I'm testing your patch on my SH4 device. When I create new partition with lz4hc compressor, it works fine: I can copy file into it, and md5sums of original and copy match. However, after reboot I cannot read the file anymore: UBIFS error (pid 1101): ubifs_decompress: cannot decompress 934 bytes, compressor lz4hc, error -22 UBIFS error (pid 1101): read_block: bad data node (block 1, inode 65) UBIFS error (pid 1101): do_readpage: cannot read page 1 of inode 65, error -22 The same error appears if I use lz4hc-compressed ubifs image to flash rootfs (using patched mkfs.ubifs). Decompression error occurs in lz4_uncompress() function (lib/lz4/lz4_decompress.c), on the line 101: /* Error: offset create reference outside destination buffer */ if (unlikely(ref (BYTE *const) dest)) goto _output_error; Brent: are you able to read data from lz4hc volume on your device? Anyone: any ideas what may happen here? -- Regards, Konstantin Konstantin, I haven't seen anything like that on my at91sam9m10g45-ek development board. I haven't used a flash image from mkfs.ubifs yet. Is it possible the file system was not umounted cleanly before the reboot and UBIFS went through a recovery procedure? Maybe something breaks with lz4hc when UBIFS does a recovery? That's just a guess. Could you save attached file on lz4hc volume, umount it and mount again? I get aforementioned error when doing `cat set11.cfg` -- Regards, Konstantin -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
