| Message ID | 20181126100253.3299-1-khorenko@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fbcon: use kvmalloc() for scrollback buffer | expand |
Hi, On 11/26/2018 11:02 AM, Konstantin Khorenko wrote: > Scrollback frame buffer is rather big - 32K, > so it requires 3rd order page, so let's use kvmalloc() instead of > ordinary kmalloc() for it. Is it actually safe to use non-contiguous memory for softback_buf? > Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com> > --- > drivers/video/fbdev/core/fbcon.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c > index 8958ccc8b1ac..2b1a34d3f5e2 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -992,7 +992,7 @@ static const char *fbcon_startup(void) > if (!softback_buf) { > softback_buf = > (unsigned long) > - kmalloc(fbcon_softback_size, > + kvmalloc(fbcon_softback_size, > GFP_KERNEL); > if (!softback_buf) { > fbcon_softback_size = 0; > @@ -1001,7 +1001,7 @@ static const char *fbcon_startup(void) > } > } else { > if (softback_buf) { > - kfree((void *) softback_buf); > + kvfree((void *) softback_buf); > softback_buf = 0; > softback_top = 0; > } > @@ -3665,7 +3665,7 @@ static void fbcon_exit(void) > } > #endif > > - kfree((void *)softback_buf); > + kvfree((void *)softback_buf); > softback_buf = 0UL; > > for_each_registered_fb(i) { Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Hi Bartlomiej, On 12/20/2018 07:21 PM, Bartlomiej Zolnierkiewicz wrote: > On 11/26/2018 11:02 AM, Konstantin Khorenko wrote: >> Scrollback frame buffer is rather big - 32K, >> so it requires 3rd order page, so let's use kvmalloc() instead of >> ordinary kmalloc() for it. > > Is it actually safe to use non-contiguous memory for softback_buf? Well, that's why we need a review. :) i've asked myself same question while fixing this, i've dig sources a bit and did not find places when softback_buf is provided for DMA, all other places seems to work with virtual addresses, so there should be no problem. Even more i saw a function which mentions that softback might be non-contigious: /* As we might be inside of softback, we may work with non-contiguous buffer, that's why we have to use a separate routine. */ static void fbcon_invert_region(struct vc_data *vc, u16 * p, int cnt) So i think it's safe to use kvmalloc() here. -- Best regards, Konstantin Khorenko, Virtuozzo Linux Kernel Team >> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com> >> --- >> drivers/video/fbdev/core/fbcon.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c >> index 8958ccc8b1ac..2b1a34d3f5e2 100644 >> --- a/drivers/video/fbdev/core/fbcon.c >> +++ b/drivers/video/fbdev/core/fbcon.c >> @@ -992,7 +992,7 @@ static const char *fbcon_startup(void) >> if (!softback_buf) { >> softback_buf = >> (unsigned long) >> - kmalloc(fbcon_softback_size, >> + kvmalloc(fbcon_softback_size, >> GFP_KERNEL); >> if (!softback_buf) { >> fbcon_softback_size = 0; >> @@ -1001,7 +1001,7 @@ static const char *fbcon_startup(void) >> } >> } else { >> if (softback_buf) { >> - kfree((void *) softback_buf); >> + kvfree((void *) softback_buf); >> softback_buf = 0; >> softback_top = 0; >> } >> @@ -3665,7 +3665,7 @@ static void fbcon_exit(void) >> } >> #endif >> >> - kfree((void *)softback_buf); >> + kvfree((void *)softback_buf); >> softback_buf = 0UL; >> >> for_each_registered_fb(i) { > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics
On 12/21/2018 11:58 AM, Konstantin Khorenko wrote: > Hi Bartlomiej, > > On 12/20/2018 07:21 PM, Bartlomiej Zolnierkiewicz wrote: >> On 11/26/2018 11:02 AM, Konstantin Khorenko wrote: >>> Scrollback frame buffer is rather big - 32K, >>> so it requires 3rd order page, so let's use kvmalloc() instead of >>> ordinary kmalloc() for it. >> >> Is it actually safe to use non-contiguous memory for softback_buf? > > Well, that's why we need a review. :) :) > i've asked myself same question while fixing this, > i've dig sources a bit and did not find places when softback_buf is provided for DMA, > all other places seems to work with virtual addresses, so there should be no problem. > > Even more i saw a function which mentions that softback might be non-contigious: > > /* As we might be inside of softback, we may work with non-contiguous buffer, > that's why we have to use a separate routine. */ > static void fbcon_invert_region(struct vc_data *vc, u16 * p, int cnt) > > So i think it's safe to use kvmalloc() here. Patch queued for v5.1, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 8958ccc8b1ac..2b1a34d3f5e2 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -992,7 +992,7 @@ static const char *fbcon_startup(void) if (!softback_buf) { softback_buf = (unsigned long) - kmalloc(fbcon_softback_size, + kvmalloc(fbcon_softback_size, GFP_KERNEL); if (!softback_buf) { fbcon_softback_size = 0; @@ -1001,7 +1001,7 @@ static const char *fbcon_startup(void) } } else { if (softback_buf) { - kfree((void *) softback_buf); + kvfree((void *) softback_buf); softback_buf = 0; softback_top = 0; } @@ -3665,7 +3665,7 @@ static void fbcon_exit(void) } #endif - kfree((void *)softback_buf); + kvfree((void *)softback_buf); softback_buf = 0UL; for_each_registered_fb(i) {
Scrollback frame buffer is rather big - 32K, so it requires 3rd order page, so let's use kvmalloc() instead of ordinary kmalloc() for it. Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com> --- drivers/video/fbdev/core/fbcon.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)