[alsa-devel] [PATCH] ALSA: Replace snd_malloc_pages() and snd_free_pages() with standard helpers
snd_malloc_pages() and snd_free_pages() are merely thin wrappers of the standard page allocator / free functions. Even the arguments are compatible with some standard helpers, so there is little merit of keeping these wrappers.
This patch replaces the all existing callers of snd_malloc_pages() and snd_free_pages() with the direct calls of the standard helper functions. In this version, we use a recently introduced one, alloc_pages_exact(), which suits better than the old snd_malloc_pages() implementation. Then we can avoid the waste of pages by alignment to power-of-two.
Also, the __GFP_COMP flag that has been always added in the old implementation is dropped for most of callers now. The only one that may still need it is the PCM buffer allocation which can be quite large. The rest are single page allocations, so no sense to keep that flag.
With these conversions, snd_malloc_pages() and snd_free_pages() are dropped altogether from the whole tree.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/memalloc.h | 4 --- sound/core/memalloc.c | 54 +++------------------------------ sound/core/pcm.c | 14 ++++----- sound/usb/usx2y/usX2Yhwdep.c | 3 +- sound/usb/usx2y/usbusx2y.c | 3 +- sound/usb/usx2y/usx2yhwdeppcm.c | 6 ++-- 6 files changed, 19 insertions(+), 65 deletions(-)
diff --git a/include/sound/memalloc.h b/include/sound/memalloc.h index af3fa577fa06..c56cb2871368 100644 --- a/include/sound/memalloc.h +++ b/include/sound/memalloc.h @@ -152,9 +152,5 @@ int snd_dma_alloc_pages_fallback(int type, struct device *dev, size_t size, struct snd_dma_buffer *dmab); void snd_dma_free_pages(struct snd_dma_buffer *dmab);
-/* basic memory allocation functions */ -void *snd_malloc_pages(size_t size, gfp_t gfp_flags); -void snd_free_pages(void *ptr, size_t size); - #endif /* __SOUND_MEMALLOC_H */
diff --git a/sound/core/memalloc.c b/sound/core/memalloc.c index 59a4adc286ed..d12bf811e751 100644 --- a/sound/core/memalloc.c +++ b/sound/core/memalloc.c @@ -30,53 +30,6 @@ #endif #include <sound/memalloc.h>
-/* - * - * Generic memory allocators - * - */ - -/** - * snd_malloc_pages - allocate pages with the given size - * @size: the size to allocate in bytes - * @gfp_flags: the allocation conditions, GFP_XXX - * - * Allocates the physically contiguous pages with the given size. - * - * Return: The pointer of the buffer, or %NULL if no enough memory. - */ -void *snd_malloc_pages(size_t size, gfp_t gfp_flags) -{ - int pg; - - if (WARN_ON(!size)) - return NULL; - if (WARN_ON(!gfp_flags)) - return NULL; - gfp_flags |= __GFP_COMP; /* compound page lets parts be mapped */ - pg = get_order(size); - return (void *) __get_free_pages(gfp_flags, pg); -} -EXPORT_SYMBOL(snd_malloc_pages); - -/** - * snd_free_pages - release the pages - * @ptr: the buffer pointer to release - * @size: the allocated buffer size - * - * Releases the buffer allocated via snd_malloc_pages(). - */ -void snd_free_pages(void *ptr, size_t size) -{ - int pg; - - if (ptr == NULL) - return; - pg = get_order(size); - free_pages((unsigned long) ptr, pg); -} -EXPORT_SYMBOL(snd_free_pages); - /* * * Bus-specific memory allocators @@ -188,8 +141,9 @@ int snd_dma_alloc_pages(int type, struct device *device, size_t size, dmab->bytes = 0; switch (type) { case SNDRV_DMA_TYPE_CONTINUOUS: - dmab->area = snd_malloc_pages(size, - (__force gfp_t)(unsigned long)device); + dmab->area = alloc_pages_exact(size, + (__force gfp_t)(unsigned long)device | + __GFP_COMP); dmab->addr = 0; break; #ifdef CONFIG_HAS_DMA @@ -273,7 +227,7 @@ void snd_dma_free_pages(struct snd_dma_buffer *dmab) { switch (dmab->dev.type) { case SNDRV_DMA_TYPE_CONTINUOUS: - snd_free_pages(dmab->area, dmab->bytes); + free_pages_exact(dmab->area, dmab->bytes); break; #ifdef CONFIG_HAS_DMA #ifdef CONFIG_GENERIC_ALLOCATOR diff --git a/sound/core/pcm.c b/sound/core/pcm.c index fdb9b92fc8d6..1c0ecb22b1f4 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -1004,22 +1004,22 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, return -ENOMEM;
size = PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status)); - runtime->status = snd_malloc_pages(size, GFP_KERNEL); + runtime->status = alloc_pages_exact(size, GFP_KERNEL); if (runtime->status == NULL) { kfree(runtime); return -ENOMEM; } - memset((void*)runtime->status, 0, size); + memset(runtime->status, 0, size);
size = PAGE_ALIGN(sizeof(struct snd_pcm_mmap_control)); - runtime->control = snd_malloc_pages(size, GFP_KERNEL); + runtime->control = alloc_pages_exact(size, GFP_KERNEL); if (runtime->control == NULL) { - snd_free_pages((void*)runtime->status, + free_pages_exact(runtime->status, PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status))); kfree(runtime); return -ENOMEM; } - memset((void*)runtime->control, 0, size); + memset(runtime->control, 0, size);
init_waitqueue_head(&runtime->sleep); init_waitqueue_head(&runtime->tsleep); @@ -1045,9 +1045,9 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream) runtime = substream->runtime; if (runtime->private_free != NULL) runtime->private_free(runtime); - snd_free_pages((void*)runtime->status, + free_pages_exact(runtime->status, PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status))); - snd_free_pages((void*)runtime->control, + free_pages_exact(runtime->control, PAGE_ALIGN(sizeof(struct snd_pcm_mmap_control))); kfree(runtime->hw_constraints.rules); /* Avoid concurrent access to runtime via PCM timer interface */ diff --git a/sound/usb/usx2y/usX2Yhwdep.c b/sound/usb/usx2y/usX2Yhwdep.c index c1dd9a7b48df..bfe1108416cf 100644 --- a/sound/usb/usx2y/usX2Yhwdep.c +++ b/sound/usb/usx2y/usX2Yhwdep.c @@ -75,7 +75,8 @@ static int snd_us428ctls_mmap(struct snd_hwdep * hw, struct file *filp, struct v
if (!us428->us428ctls_sharedmem) { init_waitqueue_head(&us428->us428ctls_wait_queue_head); - if(!(us428->us428ctls_sharedmem = snd_malloc_pages(sizeof(struct us428ctls_sharedmem), GFP_KERNEL))) + us428->us428ctls_sharedmem = alloc_pages_exact(sizeof(struct us428ctls_sharedmem), GFP_KERNEL); + if (!us428->us428ctls_sharedmem) return -ENOMEM; memset(us428->us428ctls_sharedmem, -1, sizeof(struct us428ctls_sharedmem)); us428->us428ctls_sharedmem->CtlSnapShotLast = -2; diff --git a/sound/usb/usx2y/usbusx2y.c b/sound/usb/usx2y/usbusx2y.c index da4a5a541512..9f7bbed2c0f0 100644 --- a/sound/usb/usx2y/usbusx2y.c +++ b/sound/usb/usx2y/usbusx2y.c @@ -437,7 +437,8 @@ static void snd_usX2Y_card_private_free(struct snd_card *card) kfree(usX2Y(card)->In04Buf); usb_free_urb(usX2Y(card)->In04urb); if (usX2Y(card)->us428ctls_sharedmem) - snd_free_pages(usX2Y(card)->us428ctls_sharedmem, sizeof(*usX2Y(card)->us428ctls_sharedmem)); + free_pages_exact(usX2Y(card)->us428ctls_sharedmem, + sizeof(*usX2Y(card)->us428ctls_sharedmem)); if (usX2Y(card)->card_index >= 0 && usX2Y(card)->card_index < SNDRV_CARDS) snd_usX2Y_card_used[usX2Y(card)->card_index] = 0; } diff --git a/sound/usb/usx2y/usx2yhwdeppcm.c b/sound/usb/usx2y/usx2yhwdeppcm.c index 4fd9276b8e50..4453c2b911d0 100644 --- a/sound/usb/usx2y/usx2yhwdeppcm.c +++ b/sound/usb/usx2y/usx2yhwdeppcm.c @@ -488,7 +488,9 @@ static int snd_usX2Y_usbpcm_prepare(struct snd_pcm_substream *substream) snd_printdd("snd_usX2Y_pcm_prepare(%p)\n", substream);
if (NULL == usX2Y->hwdep_pcm_shm) { - if (NULL == (usX2Y->hwdep_pcm_shm = snd_malloc_pages(sizeof(struct snd_usX2Y_hwdep_pcm_shm), GFP_KERNEL))) + usX2Y->hwdep_pcm_shm = alloc_pages_exact(sizeof(struct snd_usX2Y_hwdep_pcm_shm), + GFP_KERNEL); + if (!usX2Y->hwdep_pcm_shm) return -ENOMEM; memset(usX2Y->hwdep_pcm_shm, 0, sizeof(struct snd_usX2Y_hwdep_pcm_shm)); } @@ -700,7 +702,7 @@ static void snd_usX2Y_hwdep_pcm_private_free(struct snd_hwdep *hwdep) { struct usX2Ydev *usX2Y = hwdep->private_data; if (NULL != usX2Y->hwdep_pcm_shm) - snd_free_pages(usX2Y->hwdep_pcm_shm, sizeof(struct snd_usX2Y_hwdep_pcm_shm)); + free_pages_exact(usX2Y->hwdep_pcm_shm, sizeof(struct snd_usX2Y_hwdep_pcm_shm)); }
Hi Takashi,
I love your patch! Yet something to improve:
[auto build test ERROR on sound/for-next] [also build test ERROR on v4.20-rc3 next-20181123] [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/Takashi-Iwai/ALSA-Replace-snd_mallo... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next config: sparc64-defconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sparc64
All errors (new ones prefixed by >>):
sound/sparc/cs4231.c: In function 'snd_cs4231_playback_open':
sound/sparc/cs4231.c:1150:3: error: implicit declaration of function 'snd_free_pages'; did you mean '__free_pages'? [-Werror=implicit-function-declaration]
snd_free_pages(runtime->dma_area, runtime->dma_bytes); ^~~~~~~~~~~~~~ __free_pages cc1: some warnings being treated as errors
vim +1150 sound/sparc/cs4231.c
^1da177e Linus Torvalds 2005-04-16 1139 be9b7e8c Takashi Iwai 2006-01-03 1140 static int snd_cs4231_playback_open(struct snd_pcm_substream *substream) ^1da177e Linus Torvalds 2005-04-16 1141 { be9b7e8c Takashi Iwai 2006-01-03 1142 struct snd_cs4231 *chip = snd_pcm_substream_chip(substream); be9b7e8c Takashi Iwai 2006-01-03 1143 struct snd_pcm_runtime *runtime = substream->runtime; ^1da177e Linus Torvalds 2005-04-16 1144 int err; ^1da177e Linus Torvalds 2005-04-16 1145 ^1da177e Linus Torvalds 2005-04-16 1146 runtime->hw = snd_cs4231_playback; ^1da177e Linus Torvalds 2005-04-16 1147 9e9abb4f Krzysztof Helt 2007-09-10 1148 err = snd_cs4231_open(chip, CS4231_MODE_PLAY); 9e9abb4f Krzysztof Helt 2007-09-10 1149 if (err < 0) { ^1da177e Linus Torvalds 2005-04-16 @1150 snd_free_pages(runtime->dma_area, runtime->dma_bytes); ^1da177e Linus Torvalds 2005-04-16 1151 return err; ^1da177e Linus Torvalds 2005-04-16 1152 } ^1da177e Linus Torvalds 2005-04-16 1153 chip->playback_substream = substream; ^1da177e Linus Torvalds 2005-04-16 1154 chip->p_periods_sent = 0; ^1da177e Linus Torvalds 2005-04-16 1155 snd_pcm_set_sync(substream); ^1da177e Linus Torvalds 2005-04-16 1156 snd_cs4231_xrate(runtime); ^1da177e Linus Torvalds 2005-04-16 1157 ^1da177e Linus Torvalds 2005-04-16 1158 return 0; ^1da177e Linus Torvalds 2005-04-16 1159 } ^1da177e Linus Torvalds 2005-04-16 1160
:::::: The code at line 1150 was first introduced by commit :::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
:::::: TO: Linus Torvalds torvalds@ppc970.osdl.org :::::: CC: Linus Torvalds torvalds@ppc970.osdl.org
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Takashi,
I love your patch! Yet something to improve:
[auto build test ERROR on sound/for-next] [also build test ERROR on v4.20-rc3 next-20181123] [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/Takashi-Iwai/ALSA-Replace-snd_mallo... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next config: i386-allmodconfig (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=i386
All errors (new ones prefixed by >>):
sound/isa/wss/wss_lib.c: In function 'snd_wss_playback_open':
sound/isa/wss/wss_lib.c:1534:3: error: implicit declaration of function 'snd_free_pages'; did you mean '__free_pages'? [-Werror=implicit-function-declaration]
snd_free_pages(runtime->dma_area, runtime->dma_bytes); ^~~~~~~~~~~~~~ __free_pages cc1: some warnings being treated as errors
vim +1534 sound/isa/wss/wss_lib.c
^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1497 ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1498 */ ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1499 7779f75f sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1500 static int snd_wss_playback_open(struct snd_pcm_substream *substream) ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1501 { 7779f75f sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1502 struct snd_wss *chip = snd_pcm_substream_chip(substream); ba2375a4 sound/isa/cs423x/cs4231_lib.c Takashi Iwai 2005-11-17 1503 struct snd_pcm_runtime *runtime = substream->runtime; ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1504 int err; ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1505 7779f75f sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1506 runtime->hw = snd_wss_playback; ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1507 ead893c0 sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1508 /* hardware limitation of older chipsets */ ead893c0 sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1509 if (chip->hardware & WSS_HW_AD1848_MASK) ead893c0 sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1510 runtime->hw.formats &= ~(SNDRV_PCM_FMTBIT_IMA_ADPCM | ead893c0 sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1511 SNDRV_PCM_FMTBIT_S16_BE); ead893c0 sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1512 ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1513 /* hardware bug in InterWave chipset */ 7779f75f sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1514 if (chip->hardware == WSS_HW_INTERWAVE && chip->dma1 > 3) ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1515 runtime->hw.formats &= ~SNDRV_PCM_FMTBIT_MU_LAW; ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1516 ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1517 /* hardware limitation of cheap chips */ 7779f75f sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1518 if (chip->hardware == WSS_HW_CS4235 || 7779f75f sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1519 chip->hardware == WSS_HW_CS4239) ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1520 runtime->hw.formats = SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S16_LE; ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1521 ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1522 snd_pcm_limit_isa_dma_size(chip->dma1, &runtime->hw.buffer_bytes_max); ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1523 snd_pcm_limit_isa_dma_size(chip->dma1, &runtime->hw.period_bytes_max); ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1524 ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1525 if (chip->claim_dma) { ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1526 if ((err = chip->claim_dma(chip, chip->dma_private_data, chip->dma1)) < 0) ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1527 return err; ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1528 } ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1529 7779f75f sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1530 err = snd_wss_open(chip, WSS_MODE_PLAY); 7779f75f sound/isa/wss/wss_lib.c Krzysztof Helt 2008-07-31 1531 if (err < 0) { ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1532 if (chip->release_dma) ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1533 chip->release_dma(chip, chip->dma_private_data, chip->dma1); ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 @1534 snd_free_pages(runtime->dma_area, runtime->dma_bytes); ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1535 return err; ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1536 } ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1537 chip->playback_substream = substream; ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1538 snd_pcm_set_sync(substream); ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1539 chip->rate_constraint(runtime); ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1540 return 0; ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1541 } ^1da177e sound/isa/cs423x/cs4231_lib.c Linus Torvalds 2005-04-16 1542
:::::: The code at line 1534 was first introduced by commit :::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
:::::: TO: Linus Torvalds torvalds@ppc970.osdl.org :::::: CC: Linus Torvalds torvalds@ppc970.osdl.org
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi,
On Fri, Nov 23, 2018 at 09:35:26PM +0100, Takashi Iwai wrote:
snd_malloc_pages() and snd_free_pages() are merely thin wrappers of the standard page allocator / free functions. Even the arguments are compatible with some standard helpers, so there is little merit of keeping these wrappers.
This patch replaces the all existing callers of snd_malloc_pages() and snd_free_pages() with the direct calls of the standard helper functions. In this version, we use a recently introduced one, alloc_pages_exact(), which suits better than the old snd_malloc_pages() implementation. Then we can avoid the waste of pages by alignment to power-of-two.
Also, the __GFP_COMP flag that has been always added in the old implementation is dropped for most of callers now. The only one that may still need it is the PCM buffer allocation which can be quite large. The rest are single page allocations, so no sense to keep that flag.
With these conversions, snd_malloc_pages() and snd_free_pages() are dropped altogether from the whole tree.
Signed-off-by: Takashi Iwai tiwai@suse.de
include/sound/memalloc.h | 4 --- sound/core/memalloc.c | 54 +++------------------------------ sound/core/pcm.c | 14 ++++----- sound/usb/usx2y/usX2Yhwdep.c | 3 +- sound/usb/usx2y/usbusx2y.c | 3 +- sound/usb/usx2y/usx2yhwdeppcm.c | 6 ++-- 6 files changed, 19 insertions(+), 65 deletions(-)
Usage of 'alloc_pages_exact()' function and '__GFP_COMP' flag looks good to me.
Reviewed-by: Takashi Sakamoto o-takashi@sakamocchi.jp
Messages from 01.org can be ignored because the past patches can suppress it.
Regards
Takashi Sakamoto
participants (3)
-
kbuild test robot
-
Takashi Iwai
-
Takashi Sakamoto