RE: [PATCH] asm-generic headers: modify stat.h in include/asm-generic to be applicable to more architectures

["Guan Xuetao" <guanxuetao@mprc.pku.edu.cn>]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Tuesday, January 11, 2011 12:41 PM
> To: Guan Xuetao; David Howells
> Cc: linux-arch@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] asm-generic headers: modify stat.h in include/asm-generic to be applicable to more architectures
> 
> On Monday 10 January 2011, Guan Xuetao wrote:
> > IMO, asm-generic headers should be used in more architectures as far as possible.
> 
> They serve two purposes:
> 
> 1. To be used by other architectures (with or without being modified by arch
>    specific defines)
> 2. As an example implementation to read by people understanding the kernel or
>    porting to a new architecture.
> 
> Your patch helps the first cause, but it's counterproductive for the second.
> 
> Also, if we wanted to make all these headers fully generic, we would no longer
> need an asm-generic directory for the first use case, because then everything
> in them could be put into the include/linux/*.h headers directly. This has
> happened for a number of files in the past, but it's impractical for others.
> 
> > The patch of stat.h could be split into two parts to discuss.
> > Firstly,  STAT64_HAS_BROKEN_ST_INO is defined in most architecture's asm/stat.h,
> > and it should be considered as the part in asm-generic/stat.h.
> 
> This one is purely historical. The definition of st_ino was too short
> initially, so the field was kept in place by renamed to __st_ino
> instead. The __st_ino should however never be used by any application
> you compile now, only by the kernel in order to remain compatible with
> existing application binaries. No new architecture should need to see
> this, because there are no preexisting binaries that use this field.
> 
> I intentionally left out all the backwards-compatibility fields from
> the asm-generic headers that I wrote.
> 
> > Secondly, STAT64_PAD_BEFORE_* are misunderstanding definitions, and perhaps
> > it should use STAT64_ST_SIZE_NEED_ALIGN_64.
> 
> It's much more complicated ;-)
> 
> The padding after st_rdev is usually from glibc trying to reserve a large
> amount of space for future extensions of rdev. Padding around st_blocks
> and st_size is usually for making it possible to extend those values
> to full 64 bit when they are not already, but some architectures use
> the padding just to provide natural alignment of the words.
> 
> Almost every architecture has a slightly different definition of
> stat64, and most of them got at least one aspect slightly wrong.
> 
> The mistake that I made in the asm-generic version was that it uses 32 bit
> st_*time, which should really be 64 bit in order to avoid the year 2038
> overflow bug.
> 
> > Indeed, the macros are used for compatibility, but most architectures could make full use of
> > asm-generic headers, and new architectures could just follow the default values.
> 
> I totally agree with the idea in generical (for other headers), but in case of the
> stat.h file, it always gets more complicated than you think at first.
> David Howells worked on a new 'struct xstat' for some time, if anything we should
> just do that and keep struct stat hidden in the dark corners of the kernel.
> 
> 	Arnd

Ok, I see.
Thanks.

Guan Xuetao