On Thursday 05 November 2015 23:10:54 Amitoj Kaur Chawla wrote:
32 bit systems using 'struct timeval' will break in the year 2038, so we modify the code appropriately.
Looks good, this is ready to get posted to the driver maintainers for further review, but it would be nice to have all your bfa patches together in one series when you do that. I don't remember which ones were already ready and if there are some that need more work.
Regarding the changelog text, what you have above is roughly ok, but it's always better to start out describing mostly what the problem is and only then saying how you solve it. Let me give an example how the sentences can be improved:
This patch replaces the use of struct timeval and do_gettimeofday() with 64 bit ktime_get_seconds() since we only need to find elapsed seconds.
!We only need to find the elapsed seconds rather than absolute time, !and we only care about full seconds, so it's better to use monotonic !times, so using ktime_get_seconds() also makes the code more efficient !and more robust against a concurrent settimeofday().
Since we need monotonic time only, stats_reset_time variable does not need to be changed from u32 to u64 type.
(this one is good)
timestamp variable of type 'time64_t' is used to store monotonic time returned by ktime_get_seconds().
(this one is redundant now, you are not changing the type)
status_ok boolean variable has been introduced to remove gcc warning of uninitialised variable 'timestamp'.
!After the conversion, we get a harmless compiler warning about the !possible use of an uninitialized warning. gcc is wrong here and !the code is actually ok. Introducing a temporary status_ok variable !to store the condition avoids the warning.
As I said, it's not important for a single patch, but as you are sending a lot of patches it's better to make the texts as useful as possible.
Arnd