public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Boaz Harrosh <bharrosh@panasas.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Eric Sandeen <sandeen@sandeen.net>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Arnd Bergmann <arnd@arndb.de>,
	mfasheh@suse.com, joel.becker@oracle.com,
	linux-kernel@vger.kernel.org, xfs-masters@oss.sgi.com,
	viro@zeniv.linux.org.uk, Ankit Jain <me@ankitjain.org>,
	linux-fsdevel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	xfs@oss.sgi.com, ocfs2-devel@oss.oracle.com
Subject: Re: [xfs-masters] [PATCH] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls
Date: Sun, 01 Feb 2009 18:57:46 +0200	[thread overview]
Message-ID: <4985D48A.6090007@panasas.com> (raw)
In-Reply-To: <20090201164130.GA32276@infradead.org>

Christoph Hellwig wrote:
> The structures have been defined exactly like that in XFS (and ocfs2)
> before, and there are similar cases in other ioctls handlers.
> 
> If anyone feels like changing this in some way feel free to wade through
> the endless discussions about the pros and cons for it, but I think
> doing it in context of this patch is not helpful.

OK so ia64 gcc is broken in regard to __attribute__((packed(1))),
and it should not be used.
But clearly the programmer, Like in this patch exactly, should spell
out the hole created by padding and spell that hole out and call it
__Padding. The porgrammer thought about it, identified there is an hole,
please don't drop this information on the floor. Put it in the code so I 
don't have to break my head on it.

> Arnd Bergmann wrote:
>> > +struct space_resv {
>> > +	__s16		l_type;
>> > +	__s16		l_whence;
>> > +	__s64		l_start;
>> > +	__s64		l_len;		/* len == 0 means until end of file */
>> > +	__s32		l_sysid;
>> > +	__u32		l_pid;
>> > +	__s32		l_pad[4];	/* reserve area			    */
>> > +};
> 
> What about telling the compiler exactly what you said above, just
> to be sure we all mean the same thing. (And as documentation for new
> comers):
> 
> +struct space_resv_64 {
> +	__s16		l_type;
> +	__s16		l_whence;
> +	__u32		reserved;
change that to
	__u32		__padding_hole;
> +	__s64		l_start;
> +	__s64		l_len;		/* len == 0 means until end of file */
> +	__s32		l_sysid;
> +	__u32		l_pid;
> +	__s32		l_pad[4];	/* reserve area			    */
> +} __packed;
Drop the evil __packed but show me the padding

Thanks
Boaz

  reply	other threads:[~2009-02-01 16:57 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-28 20:59 [PATCH] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls Ankit Jain
2009-01-31  0:22 ` Andrew Morton
2009-01-31  0:38   ` Arnd Bergmann
2009-01-31  1:14     ` Andrew Morton
2009-01-31  1:48       ` Arnd Bergmann
2009-02-01  9:48         ` Boaz Harrosh
2009-02-01 10:05           ` Geert Uytterhoeven
2009-02-01 10:39             ` Boaz Harrosh
2009-02-01 10:59               ` Geert Uytterhoeven
2009-02-01 12:32                 ` Boaz Harrosh
2009-02-01 15:37                   ` [xfs-masters] " Eric Sandeen
2009-02-01 16:25                     ` Boaz Harrosh
2009-02-01 16:35                       ` Eric Sandeen
2009-02-01 16:41                         ` Christoph Hellwig
2009-02-01 16:57                           ` Boaz Harrosh [this message]
2009-02-02  0:31                             ` Arnd Bergmann
2009-02-02  8:29                               ` Boaz Harrosh
2009-02-02  8:45                                 ` Geert Uytterhoeven
2009-02-02  9:33                                   ` Boaz Harrosh
2009-02-02 20:51                                     ` Jamie Lokier
2009-02-03  7:31                                       ` Boaz Harrosh
2009-02-03 11:21                                         ` Jamie Lokier
2009-06-19 18:28 ` Christoph Hellwig
2009-06-20  8:13   ` Arnd Bergmann
2009-06-21 18:41     ` [xfs-masters] " Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4985D48A.6090007@panasas.com \
    --to=bharrosh@panasas.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=geert@linux-m68k.org \
    --cc=hch@infradead.org \
    --cc=joel.becker@oracle.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=me@ankitjain.org \
    --cc=mfasheh@suse.com \
    --cc=ocfs2-devel@oss.oracle.com \
    --cc=sandeen@sandeen.net \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xfs-masters@oss.sgi.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox