On Wed, Nov 11, 2015 at 1:22 AM, Arnd Bergmann arnd@arndb.de wrote:
On Tuesday 10 November 2015 22:21:50 Amitoj Kaur Chawla wrote:
On Tue, Nov 10, 2015 at 10:01 PM, Arnd Bergmann arnd@arndb.de wrote:
On Tuesday 10 November 2015 19:27:49 Amitoj Kaur Chawla wrote:
After looking into this further the struct bfa_itnim_ioprofile_s *ioprofile data is being used in several structures in this way:
the structure "bfa_itnim_ioprofile_s" referenced by the structure "bfa_itnim_s" referenced by the structure "bfa_s" referenced by the structure "bfad_s" referenced by bfad_im_bsg_vendor_request() function
The bfad_im_bsg_vendor_request() function is called for the case FC_BSG_HST_VENDOR in bfad_bsg.c file
Correct. So this is a vendor specific command that can get sent from user space to retrieve the profile data. We can't change that format because we don't want to break existing applications using it, so this has to be documented carefully.
An easy way out would be to leave a cast to 'time_t' in the last place that references this, to leave it up to the next person who wants to clean up the driver. Then you only do the other parts that have an easier solution.
Just to ensure that I'm understanding correctly,
So this is what we have presently: struct bfa_itnim_ioprofile_s { ... u32 io_profile_start_time; /* IO profile start time */ ... };
and I make the following change:
struct bfa_itnim_ioprofile_s { ... time_t io_profile_start_time; /* IO profile start time */ ... };
No, that would be broken on 64-bit systems because you are changing the layout of the structure. I meant adding a cast to the assignment as in
itnim->ioprofile.io_profile_start_time = (u32)(time_t)bfa_io_profile_start_time(itnim->bfa);There are many ways to do this, the above is just what I'd pick to make it obvious that this is where we lose the upper bits. In particular, this will still show up as a build error when we finally remove the time_t definition.
You could also do the same thing as a an intermediate step, and do a final patch on top that removes the time_t and explains in the changelog what the impact of removing that is.
The above change would become an intermediate change, and I send a separate(final) patch using time64_t?
The final patch would remove the time_t cast, to make it build when that is no longer defined. I'm unsure about whether we want this change to be part of the kernel though, so keeping it separate gives the maintainer the choice whether to apply it or not.
I don't get a build error when I remove the time_t cast. Where am I going wrong?