From: Michael Tokarev <mjt@tls.msk.ru>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Andi Kleen <andi@firstfloor.org>,
Linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: compat ioctl32 for /dev/snapshot?
Date: Tue, 05 May 2009 02:26:28 +0400 [thread overview]
Message-ID: <49FF6B94.2080008@msgid.tls.msk.ru> (raw)
In-Reply-To: <200905041352.18428.arnd@arndb.de>
Arnd Bergmann wrote:
> On Monday 04 May 2009, Michael Tokarev wrote:
>> Is the following patch ok? I just pulled all the SNAPSHOT_* stuff from
>> include/linux/suspend_ioctls.h and added them into fs/compat_ioctl.c.
[]
> You are however missing support for deprecated ioctls in your
> code: SNAPSHOT_SET_SWAP_FILE and SNAPSHOT_PMOPS are trivial
> (COMPATIBLE_IOCTL), but you also need to add support for
> these
Well, as comments in that file (kernel/power/user.c) states, those
ioctls are obsolete and will be removed and are only preserved for
compatibility etc. Since no one complained so far (well, no one
complained about whole /dev/snapshot thing at all ;), maybe there's
no need to define those and the following ones too?
> #define SNAPSHOT_ATOMIC_SNAPSHOT32 _IOW(SNAPSHOT_IOC_MAGIC, 3, compat_uptr_t)
> #define SNAPSHOT_SET_IMAGE_SIZE32 _IOW(SNAPSHOT_IOC_MAGIC, 6, compat_ulong)
> #define SNAPSHOT_AVAIL_SWAP32 _IOR(SNAPSHOT_IOC_MAGIC, 7, compat_uptr_t)
> #define SNAPSHOT_GET_SWAP_PAGE32 _IOR(SNAPSHOT_IOC_MAGIC, 8, compat_uptr_t)
>
>> I can't test it so far, because uswsusp tools are broken in mixed
>> 32/64bit case in other places. But at least it compiles fine and
>> does not complain anymore.
>
> I try to reduce the size of compat_ioctl.c. A better implementation
> would be to add a ->compat_ioctl() operation to the file_operations
> and list the compatible ioctl numbers as well.
Well, your way is definitely a lot cleaner. But I didn't know about
either of them anyway :)
> Please try this patch instead:
Ok. Your patch was garbled by your MUA (inserting =3D =20 and breaking
lines), but that's ok. After de-garbling it now complains about
undefined compat_uptr_t. For that, I've added
#ifdef CONFIG_COMPAT
#include <linux/compat.h>
#endif
at the end of the file. Also there was a typo (duplicated above),
compat_ulong should be compat_ulong_t. After all that it finally
compiles. As of "works" -- the above my comment still applies,
uswsusp for one is badly broken on 32/64 bits. I'm trying to
create a simpler test-case, as time permits.
Thanks!
/mjt
next prev parent reply other threads:[~2009-05-04 22:26 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-04 9:29 compat ioctl32 for /dev/snapshot? Michael Tokarev
2009-05-04 9:35 ` Andi Kleen
2009-05-04 10:55 ` Michael Tokarev
2009-05-04 11:12 ` Andi Kleen
2009-05-04 21:55 ` Rafael J. Wysocki
2009-05-05 11:38 ` Arnd Bergmann
2009-05-05 11:43 ` Michael Tokarev
2009-05-05 23:13 ` Rafael J. Wysocki
2009-05-04 11:52 ` Arnd Bergmann
2009-05-04 22:26 ` Michael Tokarev [this message]
2009-05-05 10:58 ` Arnd Bergmann
2009-05-04 21:58 ` Rafael J. Wysocki
2009-07-10 16:21 ` Pavel Machek
2009-07-12 0:19 ` Michael Tokarev
2009-07-12 15:07 ` Arnd Bergmann
2009-07-13 6:51 ` Michael Tokarev
2009-07-13 20:21 ` Pavel Machek
2009-07-14 6:57 ` Michael Tokarev
2009-07-14 9:55 ` Pavel Machek
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=49FF6B94.2080008@msgid.tls.msk.ru \
--to=mjt@tls.msk.ru \
--cc=andi@firstfloor.org \
--cc=arnd@arndb.de \
--cc=linux-kernel@vger.kernel.org \
/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