All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
@ 2015-09-30 16:10 Ksenija Stanojevic
  2015-09-30 21:52 ` [Y2038] " Arnd Bergmann
  0 siblings, 1 reply; 11+ messages in thread
From: Ksenija Stanojevic @ 2015-09-30 16:10 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: y2038, Ksenija Stanojevic

struct timespec will overflow in year 2038, so replace it with
ktime_t. And replace functions that use struct timespec,
timespec_sub with ktime_sub. Also use monotonic time instead of real
time, by replacing getnstimeofday with ktime_get, to be more robust
against leap seconds and settimeofday() calls.

Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---

Changes in v2:
        -use ktime_t instead timespec64
        -use ktime_sub instead timespec64_sub
        -use monotonic instead real-time.

 drivers/staging/fbtft/fbtft-core.c | 35 +++++++++++++----------------------
 drivers/staging/fbtft/fbtft.h      |  2 +-
 2 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 7f5fa3d..a1645e1 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -365,16 +365,15 @@ static void fbtft_update_display(struct fbtft_par *par, unsigned start_line,
 				 unsigned end_line)
 {
 	size_t offset, len;
-	struct timespec ts_start, ts_end, ts_fps, ts_duration;
-	long fps_ms, fps_us, duration_ms, duration_us;
-	long fps, throughput;
+	ktime_t ts_start, ts_end, ts_fps;
+	long  long fps, throughput;
 	bool timeit = false;
 	int ret = 0;
 
 	if (unlikely(par->debug & (DEBUG_TIME_FIRST_UPDATE | DEBUG_TIME_EACH_UPDATE))) {
 		if ((par->debug & DEBUG_TIME_EACH_UPDATE) ||
 				((par->debug & DEBUG_TIME_FIRST_UPDATE) && !par->first_update_done)) {
-			getnstimeofday(&ts_start);
+			ts_start = ktime_get();
 			timeit = true;
 		}
 	}
@@ -411,30 +410,22 @@ static void fbtft_update_display(struct fbtft_par *par, unsigned start_line,
 			__func__);
 
 	if (unlikely(timeit)) {
-		getnstimeofday(&ts_end);
-		if (par->update_time.tv_nsec == 0 && par->update_time.tv_sec == 0) {
-			par->update_time.tv_sec = ts_start.tv_sec;
-			par->update_time.tv_nsec = ts_start.tv_nsec;
-		}
-		ts_fps = timespec_sub(ts_start, par->update_time);
-		par->update_time.tv_sec = ts_start.tv_sec;
-		par->update_time.tv_nsec = ts_start.tv_nsec;
-		fps_ms = (ts_fps.tv_sec * 1000) + ((ts_fps.tv_nsec / 1000000) % 1000);
-		fps_us = (ts_fps.tv_nsec / 1000) % 1000;
-		fps = fps_ms * 1000 + fps_us;
+		ts_end = ktime_get();
+		if (par->update_time.tv64 == 0)
+			par->update_time = ts_start;
+
+		ts_fps = ktime_sub(ts_start, par->update_time);
+		par->update_time = ts_start;
+		fps = ktime_to_us(ts_fps);
 		fps = fps ? 1000000 / fps : 0;
 
-		ts_duration = timespec_sub(ts_end, ts_start);
-		duration_ms = (ts_duration.tv_sec * 1000) + ((ts_duration.tv_nsec / 1000000) % 1000);
-		duration_us = (ts_duration.tv_nsec / 1000) % 1000;
-		throughput = duration_ms * 1000 + duration_us;
+		throughput = ktime_us_delta(ts_end, ts_start);
 		throughput = throughput ? (len * 1000) / throughput : 0;
 		throughput = throughput * 1000 / 1024;
 
 		dev_info(par->info->device,
-			"Display update: %ld kB/s (%ld.%.3ld ms), fps=%ld (%ld.%.3ld ms)\n",
-			throughput, duration_ms, duration_us,
-			fps, fps_ms, fps_us);
+			"Display update: %lld kB/s, fps=%lld\n",
+			throughput, fps);
 		par->first_update_done = true;
 	}
 }
diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 7e9a506..8107f55 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -251,7 +251,7 @@ struct fbtft_par {
 	} gamma;
 	unsigned long debug;
 	bool first_update_done;
-	struct timespec update_time;
+	ktime_t update_time;
 	bool bgr;
 	void *extra;
 };
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Y2038] [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
  2015-09-30 16:10 [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t Ksenija Stanojevic
@ 2015-09-30 21:52 ` Arnd Bergmann
  2015-10-01 17:44   ` Ksenija Stanojević
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2015-09-30 21:52 UTC (permalink / raw)
  To: y2038; +Cc: Ksenija Stanojevic, outreachy-kernel

On Wednesday 30 September 2015 18:10:49 Ksenija Stanojevic wrote:
> struct timespec will overflow in year 2038, so replace it with
> ktime_t. And replace functions that use struct timespec,
> timespec_sub with ktime_sub. Also use monotonic time instead of real
> time, by replacing getnstimeofday with ktime_get, to be more robust
> against leap seconds and settimeofday() calls.
> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
> index 7f5fa3d..a1645e1 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -365,16 +365,15 @@ static void fbtft_update_display(struct fbtft_par *par, unsigned start_line,
>  				 unsigned end_line)
>  {
>  	size_t offset, len;
> -	struct timespec ts_start, ts_end, ts_fps, ts_duration;
> -	long fps_ms, fps_us, duration_ms, duration_us;
> -	long fps, throughput;
> +	ktime_t ts_start, ts_end, ts_fps;
> +	long  long fps, throughput;

Here you declare fps and throughput as 'long long', which causes problems later:

> @@ -411,30 +410,22 @@ static void fbtft_update_display(struct fbtft_par *par, unsigned start_line,
>  			__func__);
>  
>  	if (unlikely(timeit)) {
> -		getnstimeofday(&ts_end);
> -		if (par->update_time.tv_nsec == 0 && par->update_time.tv_sec == 0) {
> -			par->update_time.tv_sec = ts_start.tv_sec;
> -			par->update_time.tv_nsec = ts_start.tv_nsec;
> -		}
> -		ts_fps = timespec_sub(ts_start, par->update_time);
> -		par->update_time.tv_sec = ts_start.tv_sec;
> -		par->update_time.tv_nsec = ts_start.tv_nsec;
> -		fps_ms = (ts_fps.tv_sec * 1000) + ((ts_fps.tv_nsec / 1000000) % 1000);
> -		fps_us = (ts_fps.tv_nsec / 1000) % 1000;
> -		fps = fps_ms * 1000 + fps_us;
> +		ts_end = ktime_get();
> +		if (par->update_time.tv64 == 0)
> +			par->update_time = ts_start;

It's better not to access the 'tv64' field of the ktime_t directly, this
is supposed to be hidden. Just use ktime_to_ns() to do the same thing.

> +		ts_fps = ktime_sub(ts_start, par->update_time);
> +		par->update_time = ts_start;
> +		fps = ktime_to_us(ts_fps);

This can be written slightly simpler using the ktime_us_delta() function,
like you do below.

>  		fps = fps ? 1000000 / fps : 0;
>  
> -		ts_duration = timespec_sub(ts_end, ts_start);
> -		duration_ms = (ts_duration.tv_sec * 1000) + ((ts_duration.tv_nsec / 1000000) % 1000);
> -		duration_us = (ts_duration.tv_nsec / 1000) % 1000;
> -		throughput = duration_ms * 1000 + duration_us;
> +		throughput = ktime_us_delta(ts_end, ts_start);
>  		throughput = throughput ? (len * 1000) / throughput : 0;
>  		throughput = throughput * 1000 / 1024;

As mentioned above, throughput is a 64-bit 'long long', so the last line
of the context will result in a 64-bit division, which is not allowed in
32-bit kernels.

This is a bit hard to detect, and the most reliable way to find issues
like this is to compile the kernel for both a 32-bit target and a 64-bit
target (on x86, just turn CONFIG_64BIT on/off) to see all the compile-time
errors and warnings.

	Arnd


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Y2038] [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
  2015-09-30 21:52 ` [Y2038] " Arnd Bergmann
@ 2015-10-01 17:44   ` Ksenija Stanojević
  2015-10-01 20:05     ` Arnd Bergmann
  0 siblings, 1 reply; 11+ messages in thread
From: Ksenija Stanojević @ 2015-10-01 17:44 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: y2038, outreachy-kernel

Hi,

On Wed, Sep 30, 2015 at 11:52 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 30 September 2015 18:10:49 Ksenija Stanojevic wrote:
>> struct timespec will overflow in year 2038, so replace it with
>> ktime_t. And replace functions that use struct timespec,
>> timespec_sub with ktime_sub. Also use monotonic time instead of real
>> time, by replacing getnstimeofday with ktime_get, to be more robust
>> against leap seconds and settimeofday() calls.
>> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
>> index 7f5fa3d..a1645e1 100644
>> --- a/drivers/staging/fbtft/fbtft-core.c
>> +++ b/drivers/staging/fbtft/fbtft-core.c
>> @@ -365,16 +365,15 @@ static void fbtft_update_display(struct fbtft_par *par, unsigned start_line,
>>                                unsigned end_line)
>>  {
>>       size_t offset, len;
>> -     struct timespec ts_start, ts_end, ts_fps, ts_duration;
>> -     long fps_ms, fps_us, duration_ms, duration_us;
>> -     long fps, throughput;
>> +     ktime_t ts_start, ts_end, ts_fps;
>> +     long  long fps, throughput;
>
> Here you declare fps and throughput as 'long long', which causes problems later:
>
>> @@ -411,30 +410,22 @@ static void fbtft_update_display(struct fbtft_par *par, unsigned start_line,
>>                       __func__);
>>
>>       if (unlikely(timeit)) {
>> -             getnstimeofday(&ts_end);
>> -             if (par->update_time.tv_nsec == 0 && par->update_time.tv_sec == 0) {
>> -                     par->update_time.tv_sec = ts_start.tv_sec;
>> -                     par->update_time.tv_nsec = ts_start.tv_nsec;
>> -             }
>> -             ts_fps = timespec_sub(ts_start, par->update_time);
>> -             par->update_time.tv_sec = ts_start.tv_sec;
>> -             par->update_time.tv_nsec = ts_start.tv_nsec;
>> -             fps_ms = (ts_fps.tv_sec * 1000) + ((ts_fps.tv_nsec / 1000000) % 1000);
>> -             fps_us = (ts_fps.tv_nsec / 1000) % 1000;
>> -             fps = fps_ms * 1000 + fps_us;
>> +             ts_end = ktime_get();
>> +             if (par->update_time.tv64 == 0)
>> +                     par->update_time = ts_start;
>
> It's better not to access the 'tv64' field of the ktime_t directly, this
> is supposed to be hidden. Just use ktime_to_ns() to do the same thing.
>
>> +             ts_fps = ktime_sub(ts_start, par->update_time);
>> +             par->update_time = ts_start;
>> +             fps = ktime_to_us(ts_fps);
>
> This can be written slightly simpler using the ktime_us_delta() function,
> like you do below.
>
>>               fps = fps ? 1000000 / fps : 0;
>>
>> -             ts_duration = timespec_sub(ts_end, ts_start);
>> -             duration_ms = (ts_duration.tv_sec * 1000) + ((ts_duration.tv_nsec / 1000000) % 1000);
>> -             duration_us = (ts_duration.tv_nsec / 1000) % 1000;
>> -             throughput = duration_ms * 1000 + duration_us;
>> +             throughput = ktime_us_delta(ts_end, ts_start);
>>               throughput = throughput ? (len * 1000) / throughput : 0;
>>               throughput = throughput * 1000 / 1024;
>
> As mentioned above, throughput is a 64-bit 'long long', so the last line
> of the context will result in a 64-bit division, which is not allowed in
> 32-bit kernels.
>
> This is a bit hard to detect, and the most reliable way to find issues
> like this is to compile the kernel for both a 32-bit target and a 64-bit
> target (on x86, just turn CONFIG_64BIT on/off) to see all the compile-time
> errors and warnings.

In my local repository I modified my .config file so that CONFIG_64BIT
is not set,
and after that I recompiled all directory, but I don't get any errors/warning at
compile-time.
Also I separetly compiled this specific file but still no warnings
My .config  looks something like this:

#
# Automatically generated file; DO NOT EDIT.
# Linux/x86 4.3.0-rc3 Kernel Configuration
#
# CONFIG_64BIT is not set
CONFIG_X86_32=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_PERF_EVENTS_INTEL_UNCORE=y
CONFIG_OUTPUT_FORMAT="elf32-i386"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/i386_defconfig"
. . .

Should I change my working kernel or .config file is just enough?

Thanks,
Ksenija

>         Arnd


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Y2038] [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
  2015-10-01 17:44   ` Ksenija Stanojević
@ 2015-10-01 20:05     ` Arnd Bergmann
  2015-10-03 19:15       ` Ksenija Stanojević
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2015-10-01 20:05 UTC (permalink / raw)
  To: y2038; +Cc: Ksenija Stanojević, outreachy-kernel

On Thursday 01 October 2015 19:44:46 Ksenija Stanojević wrote:
> 
> In my local repository I modified my .config file so that CONFIG_64BIT
> is not set,
> and after that I recompiled all directory, but I don't get any errors/warning at
> compile-time.
> Also I separetly compiled this specific file but still no warnings
> My .config  looks something like this:
> 
> . . .
> 
> Should I change my working kernel or .config file is just enough?

I think the problem is that you only compiled that directory but did
not attempt to do a full rebuild of the kernel and modules, which is
required to catch link-time errors.

The compiler does not know at this point that the 64-bit division function
is undefined in the kernel, you only get a warning at the 'make vmlinux'
link stage (for built-in drivers) or the 'make modules' modpost stage
afterwards.

	Arnd


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Y2038] [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
  2015-10-01 20:05     ` Arnd Bergmann
@ 2015-10-03 19:15       ` Ksenija Stanojević
  2015-10-04 19:34         ` Arnd Bergmann
  0 siblings, 1 reply; 11+ messages in thread
From: Ksenija Stanojević @ 2015-10-03 19:15 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: y2038, outreachy-kernel

On Thu, Oct 1, 2015 at 10:05 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 01 October 2015 19:44:46 Ksenija Stanojević wrote:
>>
>> In my local repository I modified my .config file so that CONFIG_64BIT
>> is not set,
>> and after that I recompiled all directory, but I don't get any errors/warning at
>> compile-time.
>> Also I separetly compiled this specific file but still no warnings
>> My .config  looks something like this:
>>
>> . . .
>>
>> Should I change my working kernel or .config file is just enough?
>
> I think the problem is that you only compiled that directory but did
> not attempt to do a full rebuild of the kernel and modules, which is
> required to catch link-time errors.
>
> The compiler does not know at this point that the 64-bit division function
> is undefined in the kernel, you only get a warning at the 'make vmlinux'
> link stage (for built-in drivers) or the 'make modules' modpost stage
> afterwards.

I rebuilded my repository with:
make vmlinux
make modules
but I still don't get any warnings.
Do you have any other suggestion on what I'm doing wrong?

Thanks,
Ksenija


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Y2038] [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
  2015-10-03 19:15       ` Ksenija Stanojević
@ 2015-10-04 19:34         ` Arnd Bergmann
  2015-10-05  5:17           ` [Outreachy kernel] " Sudip Mukherjee
  2015-10-07 18:46           ` Ksenija Stanojević
  0 siblings, 2 replies; 11+ messages in thread
From: Arnd Bergmann @ 2015-10-04 19:34 UTC (permalink / raw)
  To: Ksenija Stanojević; +Cc: y2038, outreachy-kernel

On Saturday 03 October 2015 21:15:46 Ksenija Stanojević wrote:
> On Thu, Oct 1, 2015 at 10:05 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 01 October 2015 19:44:46 Ksenija Stanojević wrote:
> >>
> >> In my local repository I modified my .config file so that CONFIG_64BIT
> >> is not set,
> >> and after that I recompiled all directory, but I don't get any errors/warning at
> >> compile-time.
> >> Also I separetly compiled this specific file but still no warnings
> >> My .config  looks something like this:
> >>
> >> . . .
> >>
> >> Should I change my working kernel or .config file is just enough?
> >
> > I think the problem is that you only compiled that directory but did
> > not attempt to do a full rebuild of the kernel and modules, which is
> > required to catch link-time errors.
> >
> > The compiler does not know at this point that the 64-bit division function
> > is undefined in the kernel, you only get a warning at the 'make vmlinux'
> > link stage (for built-in drivers) or the 'make modules' modpost stage
> > afterwards.
> 
> I rebuilded my repository with:
> make vmlinux
> make modules
> but I still don't get any warnings.
> Do you have any other suggestion on what I'm doing wrong?

I just tried it on my machine and I get (for an ARM build)

$ make -skj30   
ERROR: "__aeabi_ldivmod" [drivers/staging/fbtft/fbtft.ko] undefined!

Two possible explanations why you don't get it:

- your .config file got changed back to CONFIG_64BIT being enabled
- you don't have CONFIG_FB_TFT enabled in this build.

	Arnd


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Outreachy kernel] Re: [Y2038] [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
  2015-10-04 19:34         ` Arnd Bergmann
@ 2015-10-05  5:17           ` Sudip Mukherjee
  2015-10-05  8:20             ` Arnd Bergmann
  2015-10-07 18:46           ` Ksenija Stanojević
  1 sibling, 1 reply; 11+ messages in thread
From: Sudip Mukherjee @ 2015-10-05  5:17 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Ksenija Stanojević, y2038, outreachy-kernel

On Sun, Oct 04, 2015 at 09:34:40PM +0200, Arnd Bergmann wrote:
> On Saturday 03 October 2015 21:15:46 Ksenija Stanojević wrote:
> > On Thu, Oct 1, 2015 at 10:05 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Thursday 01 October 2015 19:44:46 Ksenija Stanojević wrote:
> > >>
> > >> In my local repository I modified my .config file so that CONFIG_64BIT
> > >> is not set,
> > >> and after that I recompiled all directory, but I don't get any errors/warning at
> > >> compile-time.
> > >> Also I separetly compiled this specific file but still no warnings
> > >> My .config  looks something like this:
> > >>
> > >> . . .
> > >>
> > >> Should I change my working kernel or .config file is just enough?
> > >
> > > I think the problem is that you only compiled that directory but did
> > > not attempt to do a full rebuild of the kernel and modules, which is
> > > required to catch link-time errors.
> > >
> > > The compiler does not know at this point that the 64-bit division function
> > > is undefined in the kernel, you only get a warning at the 'make vmlinux'
> > > link stage (for built-in drivers) or the 'make modules' modpost stage
> > > afterwards.
> > 
> > I rebuilded my repository with:
> > make vmlinux
> > make modules
> > but I still don't get any warnings.
> > Do you have any other suggestion on what I'm doing wrong?
> 
> I just tried it on my machine and I get (for an ARM build)
> 
> $ make -skj30   
> ERROR: "__aeabi_ldivmod" [drivers/staging/fbtft/fbtft.ko] undefined!
> 
> Two possible explanations why you don't get it:
> 
> - your .config file got changed back to CONFIG_64BIT being enabled
> - you don't have CONFIG_FB_TFT enabled in this build.

After modifying manually .config file use make oldconfig and then make
prepare.
I think i missed the beginning of the thread. Are you saying that fbtft
build fails on 32 bit arch?

regards
sudip


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Outreachy kernel] Re: [Y2038] [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
  2015-10-05  5:17           ` [Outreachy kernel] " Sudip Mukherjee
@ 2015-10-05  8:20             ` Arnd Bergmann
  2015-10-05 14:52               ` Sudip Mukherjee
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2015-10-05  8:20 UTC (permalink / raw)
  To: Sudip Mukherjee; +Cc: Ksenija Stanojević, y2038, outreachy-kernel

On Monday 05 October 2015 10:47:05 Sudip Mukherjee wrote:
> On Sun, Oct 04, 2015 at 09:34:40PM +0200, Arnd Bergmann wrote:
> > On Saturday 03 October 2015 21:15:46 Ksenija Stanojević wrote:
> > Two possible explanations why you don't get it:
> > 
> > - your .config file got changed back to CONFIG_64BIT being enabled
> > - you don't have CONFIG_FB_TFT enabled in this build.
> 
> After modifying manually .config file use make oldconfig and then make
> prepare.
> I think i missed the beginning of the thread. Are you saying that fbtft
> build fails on 32 bit arch?

Ksenija's patch from last week causes this build regression, but it is not
merged yet. This is my original comment:

| > -             ts_duration = timespec_sub(ts_end, ts_start);
| > -             duration_ms = (ts_duration.tv_sec * 1000) + ((ts_duration.tv_nsec / 1000000) % 1000);
| > -             duration_us = (ts_duration.tv_nsec / 1000) % 1000;
| > -             throughput = duration_ms * 1000 + duration_us;
| > +             throughput = ktime_us_delta(ts_end, ts_start);
| >               throughput = throughput ? (len * 1000) / throughput : 0;
| >               throughput = throughput * 1000 / 1024;
| 
| As mentioned above, throughput is a 64-bit 'long long', so the last line
| of the context will result in a 64-bit division, which is not allowed in
| 32-bit kernels.

	Arnd


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Outreachy kernel] Re: [Y2038] [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
  2015-10-05  8:20             ` Arnd Bergmann
@ 2015-10-05 14:52               ` Sudip Mukherjee
  0 siblings, 0 replies; 11+ messages in thread
From: Sudip Mukherjee @ 2015-10-05 14:52 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Ksenija Stanojević, y2038, outreachy-kernel

On Mon, Oct 05, 2015 at 10:20:12AM +0200, Arnd Bergmann wrote:
> On Monday 05 October 2015 10:47:05 Sudip Mukherjee wrote:
> > On Sun, Oct 04, 2015 at 09:34:40PM +0200, Arnd Bergmann wrote:
> > > On Saturday 03 October 2015 21:15:46 Ksenija Stanojević wrote:
> > > Two possible explanations why you don't get it:
> > > 
> > > - your .config file got changed back to CONFIG_64BIT being enabled
> > > - you don't have CONFIG_FB_TFT enabled in this build.
> > 
> > After modifying manually .config file use make oldconfig and then make
> > prepare.
> > I think i missed the beginning of the thread. Are you saying that fbtft
> > build fails on 32 bit arch?
> 
> Ksenija's patch from last week causes this build regression, but it is not
> merged yet.
Ohhh .. ok. thanks. I have also checked the thread from my archieve now.

regards
sudip


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Y2038] [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
  2015-10-04 19:34         ` Arnd Bergmann
  2015-10-05  5:17           ` [Outreachy kernel] " Sudip Mukherjee
@ 2015-10-07 18:46           ` Ksenija Stanojević
  2015-10-07 19:08             ` Arnd Bergmann
  1 sibling, 1 reply; 11+ messages in thread
From: Ksenija Stanojević @ 2015-10-07 18:46 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: y2038, outreachy-kernel

On Sun, Oct 4, 2015 at 9:34 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 03 October 2015 21:15:46 Ksenija Stanojević wrote:
>> On Thu, Oct 1, 2015 at 10:05 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Thursday 01 October 2015 19:44:46 Ksenija Stanojević wrote:
>> >>
>> >> In my local repository I modified my .config file so that CONFIG_64BIT
>> >> is not set,
>> >> and after that I recompiled all directory, but I don't get any errors/warning at
>> >> compile-time.
>> >> Also I separetly compiled this specific file but still no warnings
>> >> My .config  looks something like this:
>> >>
>> >> . . .
>> >>
>> >> Should I change my working kernel or .config file is just enough?
>> >
>> > I think the problem is that you only compiled that directory but did
>> > not attempt to do a full rebuild of the kernel and modules, which is
>> > required to catch link-time errors.
>> >
>> > The compiler does not know at this point that the 64-bit division function
>> > is undefined in the kernel, you only get a warning at the 'make vmlinux'
>> > link stage (for built-in drivers) or the 'make modules' modpost stage
>> > afterwards.
>>
>> I rebuilded my repository with:
>> make vmlinux
>> make modules
>> but I still don't get any warnings.
>> Do you have any other suggestion on what I'm doing wrong?
>
> I just tried it on my machine and I get (for an ARM build)
>
> $ make -skj30
> ERROR: "__aeabi_ldivmod" [drivers/staging/fbtft/fbtft.ko] undefined!

I got this error:
ERROR: "__divdi3" [drivers/staging/fbtft/fbtft.ko] undefined!

I suppose that divdi3 is division used for long long by gcc on x86


Ksenija


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Y2038] [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t
  2015-10-07 18:46           ` Ksenija Stanojević
@ 2015-10-07 19:08             ` Arnd Bergmann
  0 siblings, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2015-10-07 19:08 UTC (permalink / raw)
  To: y2038; +Cc: Ksenija Stanojević, outreachy-kernel

On Wednesday 07 October 2015 20:46:23 Ksenija Stanojević wrote:
> On Sun, Oct 4, 2015 at 9:34 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Saturday 03 October 2015 21:15:46 Ksenija Stanojević wrote:
> >> On Thu, Oct 1, 2015 at 10:05 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Thursday 01 October 2015 19:44:46 Ksenija Stanojević wrote:
> >> >>
> >> >> In my local repository I modified my .config file so that CONFIG_64BIT
> >> >> is not set,
> >> >> and after that I recompiled all directory, but I don't get any errors/warning at
> >> >> compile-time.
> >> >> Also I separetly compiled this specific file but still no warnings
> >> >> My .config  looks something like this:
> >> >>
> >> >> . . .
> >> >>
> >> >> Should I change my working kernel or .config file is just enough?
> >> >
> >> > I think the problem is that you only compiled that directory but did
> >> > not attempt to do a full rebuild of the kernel and modules, which is
> >> > required to catch link-time errors.
> >> >
> >> > The compiler does not know at this point that the 64-bit division function
> >> > is undefined in the kernel, you only get a warning at the 'make vmlinux'
> >> > link stage (for built-in drivers) or the 'make modules' modpost stage
> >> > afterwards.
> >>
> >> I rebuilded my repository with:
> >> make vmlinux
> >> make modules
> >> but I still don't get any warnings.
> >> Do you have any other suggestion on what I'm doing wrong?
> >
> > I just tried it on my machine and I get (for an ARM build)
> >
> > $ make -skj30
> > ERROR: "__aeabi_ldivmod" [drivers/staging/fbtft/fbtft.ko] undefined!
> 
> I got this error:
> ERROR: "__divdi3" [drivers/staging/fbtft/fbtft.ko] undefined!
> 
> I suppose that divdi3 is division used for long long by gcc on x86

Correct, this function name is architecture specific. These
functions (__divdi3, __aeabi_ldivmod, ...) are intentionally not
part of the kernel image so force kernel developers to either
use the div_u64() family of functions for division or find a way
to avoid it, which is usually preferred but not always possible.

	Arnd


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-10-07 19:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-30 16:10 [PATCH v2] Staging: fbtbt: Replace timespec with ktime_t Ksenija Stanojevic
2015-09-30 21:52 ` [Y2038] " Arnd Bergmann
2015-10-01 17:44   ` Ksenija Stanojević
2015-10-01 20:05     ` Arnd Bergmann
2015-10-03 19:15       ` Ksenija Stanojević
2015-10-04 19:34         ` Arnd Bergmann
2015-10-05  5:17           ` [Outreachy kernel] " Sudip Mukherjee
2015-10-05  8:20             ` Arnd Bergmann
2015-10-05 14:52               ` Sudip Mukherjee
2015-10-07 18:46           ` Ksenija Stanojević
2015-10-07 19:08             ` Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.