All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
To: petkovbb@gmail.com
Cc: Nishanth Aravamudan <nacc@us.ibm.com>, Robin Holt <holt@sgi.com>,
	tony.luck@intel.com, linux-ia64@vger.kernel.org,
	linux-ide@vger.kernel.org,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: Re: kernel unaligned accesses on IA64 in IDE
Date: Fri, 22 Aug 2008 18:36:00 +0000	[thread overview]
Message-ID: <200808222036.00536.bzolnier@gmail.com> (raw)
In-Reply-To: <9ea470500808221029l6bd79c62w4c06d948f962d95c@mail.gmail.com>

On Friday 22 August 2008, Boris Petkov wrote:
> On Fri, Aug 22, 2008 at 6:45 PM, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> > On 22.08.2008 [12:55:25 +0200], Boris Petkov wrote:
> >> On Fri, Aug 22, 2008 at 12:15 PM, Bartlomiej Zolnierkiewicz
> >> <bzolnier@gmail.com> wrote:
> >> > On Friday 22 August 2008, Nishanth Aravamudan wrote:
> >> >> On 21.08.2008 [16:54:26 -0500], Robin Holt wrote:
> >> >> > > [   32.597792] outsl(496, e000000644678466, 3)
> >> >> >                             ^^^^^^^^^^^^^^^^
> >> >> >
> >> >> > This is expected to be an unsigned int * and typecast to that in outsl.
> >> >> > Looks like the buffer being passed in is not properly aligned.  Time to
> >> >> > go look at the caller.  Make sure buf is defined as an array of at least
> >> >> > int size.  That should make this aligned on a 4 byte boundary instead of
> >> >> > the 2 byte boundary it is on now.
> >> >> >
> >> >> > You can cheat at finding the callers by putting
> >> >> >     WARN_ON(buf & 0x3);
> >> >> >     printk...
> >> >>
> >> >> So I tried this and it gets quite hairy quickly (I think) because what's
> >> >> unaligned is an IDE command buffer? There is a lot of pointer passing
> >> >> and I get lost since I don't know the IDE/elevator code very well.
> >> >>
> >> >> Here's the stack trace I'm looking at:
> >> >>
> >> >> [    5.018347]  [<a000000100015420>] show_stack+0x80/0xa0
> >> >> [    5.018348]                                 spà0000130307f930 bspà000013030793b8
> >> >> [    5.031782]  [<a000000100015470>] dump_stack+0x30/0x60
> >> >> [    5.031783]                                 spà0000130307fb00 bspà000013030793a0
> >> >> [    5.045223]  [<a000000100094ff0>] warn_on_slowpath+0x90/0xe0
> >> >> [    5.045225]                                 spà0000130307fb00 bspà00001303079378
> >> >> [    5.059201]  [<a000000100517480>] ide_output_data+0x3c0/0x540
> >> >> [    5.059204]                                 spà0000130307fbf0 bspà00001303079310
> >> >> [    5.073248]  [<a0000001005309e0>] cdrom_transfer_packet_command+0x2c0/0x340
> >> >> [    5.073249]                                 spà0000130307fbf0 bspà000013030792d0
> >> >> [    5.088519]  [<a000000100530ac0>] cdrom_do_newpc_cont+0x60/0x80
> >> >> [    5.088522]                                 spà0000130307fc00 bspà000013030792b0
> >> >> [    5.102739]  [<a00000010052f1a0>] ide_cd_do_request+0x980/0x1420
> >> >> [    5.102742]                                 spà0000130307fc00 bspà00001303079238
> >> >> [    5.117064]  [<a00000010050fe00>] ide_do_request+0xca0/0x1d00
> >> >> [    5.117066]                                 spà0000130307fc00 bspà000013030791a0
> >> >> [    5.131105]  [<a000000100511580>] do_ide_request+0x40/0x60
> >> >> [    5.131107]                                 spà0000130307fc30 bspà00001303079180
> >> >> [    5.144897]  [<a000000100384780>] elv_insert+0x280/0x5c0
> >> >> [    5.144900]                                 spà0000130307fc30 bspà00001303079148
> >> >> [    5.158507]  [<a000000100384c40>] __elv_add_request+0x180/0x240
> >> >> [    5.158509]                                 spà0000130307fc30 bspà00001303079110
> >> >> [    5.172733]  [<a000000100391730>] blk_execute_rq_nowait+0xd0/0x1e0
> >> >> [    5.172734]                                 spà0000130307fc30 bspà000013030790d0
> >> >> [    5.187220]  [<a000000100391910>] blk_execute_rq+0xd0/0x240
> >> >> [    5.187221]                                 spà0000130307fc30 bspà00001303079090
> >> >> [    5.201091]  [<a000000100531f70>] ide_cd_queue_pc+0x130/0x2e0
> >> >> [    5.201093]                                 spà0000130307fcc0 bspà00001303078fd0
> >> >> [    5.215137]  [<a0000001005342f0>] ide_cdrom_packet+0x130/0x180
> >> >> [    5.215139]                                 spà0000130307fd00 bspà00001303078f78
> >> >> [    5.229281]  [<a000000100593080>] cdrom_mode_sense+0xc0/0xe0
> >> >> [    5.229283]                                 spà0000130307fd10 bspà00001303078f40
> >> >> [    5.243239]  [<a00000010052d9c0>] ide_cdrom_get_capabilities+0x80/0xc0
> >> >> [    5.243240]                                 spà0000130307fd10 bspà00001303078f10
> >> >> [    5.258084]  [<a000000100533890>] ide_cd_probe+0x810/0xf40
> >> >> [    5.258086]                                 spà0000130307fd50 bspà00001303078e90
> >> >> [    5.273709]  [<a00000010050a510>] generic_ide_probe+0x70/0xa0
> >> >> [    5.273711]                                 spà0000130307fdc0 bspà00001303078e70
> >> >> [    5.287774]  [<a0000001004bdaf0>] driver_probe_device+0x190/0x3a0
> >> >> [    5.287775]                                 spà0000130307fdc0 bspà00001303078e28
> >> >> [    5.302163]  [<a0000001004bdd80>] __driver_attach+0x80/0xe0
> >> >> [    5.302164]                                 spà0000130307fdc0 bspà00001303078de8
> >> >> [    5.316032]  [<a0000001004bc5a0>] bus_for_each_dev+0xc0/0x140
> >> >> [    5.316034]                                 spà0000130307fdc0 bspà00001303078db0
> >> >> [    5.330072]  [<a0000001004bd700>] driver_attach+0x40/0x60
> >> >> [    5.330074]                                 spà0000130307fde0 bspà00001303078d90
> >> >> [    5.343761]  [<a0000001004bd290>] bus_add_driver+0x370/0x4a0
> >> >> [    5.343763]                                 spà0000130307fde0 bspà00001303078d48
> >> >> [    5.357720]  [<a0000001004be3d0>] driver_register+0xd0/0x340
> >> >> [    5.357721]                                 spà0000130307fde0 bspà00001303078d00
> >> >> [    5.371693]  [<a0000001009a3ea0>] ide_cdrom_init+0x20/0x40
> >> >> [    5.371695]                                 spà0000130307fde0 bspà00001303078ce8
> >> >> [    5.385475]  [<a00000010000a5a0>] do_one_initcall+0x60/0x380
> >> >> [    5.385477]                                 spà0000130307fde0 bspà00001303078ca8
> >> >> [    5.399445]  [<a0000001009645b0>] kernel_init+0x370/0x420
> >> >> [    5.399447]                                 spà0000130307fe20 bspà00001303078c68
> >> >> [    5.413148]  [<a000000100013590>] kernel_thread_helper+0xd0/0x100
> >> >> [    5.413149]                                 spà0000130307fe30 bspà00001303078c40
> >> >> [    5.427547]  [<a00000010000a4c0>] start_kernel_thread+0x20/0x40
> >> >> [    5.427548]                                 spà0000130307fe30 bspà00001303078c40
> >> >>
> >> >> We are trying to send a sense command to the device and the buffer we
> >> >> use (which is rq->cmd) is what is unaligned, I believe. I'm not sure how
> >> >> useful I can be going forward...
> >> >
> >> > Borislav/Fujita, any ideas what is going wrong with ide-cd?
> >> >
> >>
> >> I think its the following:
> >>
> >> ide_cdrom_get_capabilities() allocates a struct packet_command cgc on
> >> the stack in order to do cdrom_mode_sense() later on. Since that cmd
> >> is not 4byte aligned as we've seen above and we don't do the alignment
> >> check in ide_cd_queue_pc() similar to cdrom_do_block_pc() (see
> >> 0b6abc17700a7843b165c677da0ac94522f83083), we bust the transfer later.
> >>
> >> I'll cook up something later when I have the time...
> >
> > I'm happy to test any patches (and it should be relatively quick to
> > test).
> 
> Sure, however, the problem is more hairy than I expected - the rq->cmd buffer
> is received through blk_get_request() and it is sometimes 2 bytes aligned and
> not 4. Bart, is it a sensible approach to do an ia64 ifdef that checks the
> alignment of the buffer and turns off drive->io_32bit which is checked in
> ide_output_data() thus doing only out(w|b) accesses on ia64 with misaligned
> buffers?

Well, it sounds kind of like a hack if I have to answer honestly. ;-)

Why not just make rq->cmd always properly aligned in the block layer
i.e. by allocating BLK_MAX_CDB + 2 instead of BLK_MAX_CDB for rq->__cmd
and then doing ALIGN() when setting rq->cmd in blk_rq_init()?

[ I'm sure there exists even better solution... ]

Thanks,
Bart

WARNING: multiple messages have this Message-ID (diff)
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
To: petkovbb@gmail.com
Cc: Nishanth Aravamudan <nacc@us.ibm.com>, Robin Holt <holt@sgi.com>,
	tony.luck@intel.com, linux-ia64@vger.kernel.org,
	linux-ide@vger.kernel.org,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: Re: kernel unaligned accesses on IA64 in IDE
Date: Fri, 22 Aug 2008 20:36:00 +0200	[thread overview]
Message-ID: <200808222036.00536.bzolnier@gmail.com> (raw)
In-Reply-To: <9ea470500808221029l6bd79c62w4c06d948f962d95c@mail.gmail.com>

On Friday 22 August 2008, Boris Petkov wrote:
> On Fri, Aug 22, 2008 at 6:45 PM, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> > On 22.08.2008 [12:55:25 +0200], Boris Petkov wrote:
> >> On Fri, Aug 22, 2008 at 12:15 PM, Bartlomiej Zolnierkiewicz
> >> <bzolnier@gmail.com> wrote:
> >> > On Friday 22 August 2008, Nishanth Aravamudan wrote:
> >> >> On 21.08.2008 [16:54:26 -0500], Robin Holt wrote:
> >> >> > > [   32.597792] outsl(496, e000000644678466, 3)
> >> >> >                             ^^^^^^^^^^^^^^^^
> >> >> >
> >> >> > This is expected to be an unsigned int * and typecast to that in outsl.
> >> >> > Looks like the buffer being passed in is not properly aligned.  Time to
> >> >> > go look at the caller.  Make sure buf is defined as an array of at least
> >> >> > int size.  That should make this aligned on a 4 byte boundary instead of
> >> >> > the 2 byte boundary it is on now.
> >> >> >
> >> >> > You can cheat at finding the callers by putting
> >> >> >     WARN_ON(buf & 0x3);
> >> >> >     printk...
> >> >>
> >> >> So I tried this and it gets quite hairy quickly (I think) because what's
> >> >> unaligned is an IDE command buffer? There is a lot of pointer passing
> >> >> and I get lost since I don't know the IDE/elevator code very well.
> >> >>
> >> >> Here's the stack trace I'm looking at:
> >> >>
> >> >> [    5.018347]  [<a000000100015420>] show_stack+0x80/0xa0
> >> >> [    5.018348]                                 sp=e00000130307f930 bsp=e0000013030793b8
> >> >> [    5.031782]  [<a000000100015470>] dump_stack+0x30/0x60
> >> >> [    5.031783]                                 sp=e00000130307fb00 bsp=e0000013030793a0
> >> >> [    5.045223]  [<a000000100094ff0>] warn_on_slowpath+0x90/0xe0
> >> >> [    5.045225]                                 sp=e00000130307fb00 bsp=e000001303079378
> >> >> [    5.059201]  [<a000000100517480>] ide_output_data+0x3c0/0x540
> >> >> [    5.059204]                                 sp=e00000130307fbf0 bsp=e000001303079310
> >> >> [    5.073248]  [<a0000001005309e0>] cdrom_transfer_packet_command+0x2c0/0x340
> >> >> [    5.073249]                                 sp=e00000130307fbf0 bsp=e0000013030792d0
> >> >> [    5.088519]  [<a000000100530ac0>] cdrom_do_newpc_cont+0x60/0x80
> >> >> [    5.088522]                                 sp=e00000130307fc00 bsp=e0000013030792b0
> >> >> [    5.102739]  [<a00000010052f1a0>] ide_cd_do_request+0x980/0x1420
> >> >> [    5.102742]                                 sp=e00000130307fc00 bsp=e000001303079238
> >> >> [    5.117064]  [<a00000010050fe00>] ide_do_request+0xca0/0x1d00
> >> >> [    5.117066]                                 sp=e00000130307fc00 bsp=e0000013030791a0
> >> >> [    5.131105]  [<a000000100511580>] do_ide_request+0x40/0x60
> >> >> [    5.131107]                                 sp=e00000130307fc30 bsp=e000001303079180
> >> >> [    5.144897]  [<a000000100384780>] elv_insert+0x280/0x5c0
> >> >> [    5.144900]                                 sp=e00000130307fc30 bsp=e000001303079148
> >> >> [    5.158507]  [<a000000100384c40>] __elv_add_request+0x180/0x240
> >> >> [    5.158509]                                 sp=e00000130307fc30 bsp=e000001303079110
> >> >> [    5.172733]  [<a000000100391730>] blk_execute_rq_nowait+0xd0/0x1e0
> >> >> [    5.172734]                                 sp=e00000130307fc30 bsp=e0000013030790d0
> >> >> [    5.187220]  [<a000000100391910>] blk_execute_rq+0xd0/0x240
> >> >> [    5.187221]                                 sp=e00000130307fc30 bsp=e000001303079090
> >> >> [    5.201091]  [<a000000100531f70>] ide_cd_queue_pc+0x130/0x2e0
> >> >> [    5.201093]                                 sp=e00000130307fcc0 bsp=e000001303078fd0
> >> >> [    5.215137]  [<a0000001005342f0>] ide_cdrom_packet+0x130/0x180
> >> >> [    5.215139]                                 sp=e00000130307fd00 bsp=e000001303078f78
> >> >> [    5.229281]  [<a000000100593080>] cdrom_mode_sense+0xc0/0xe0
> >> >> [    5.229283]                                 sp=e00000130307fd10 bsp=e000001303078f40
> >> >> [    5.243239]  [<a00000010052d9c0>] ide_cdrom_get_capabilities+0x80/0xc0
> >> >> [    5.243240]                                 sp=e00000130307fd10 bsp=e000001303078f10
> >> >> [    5.258084]  [<a000000100533890>] ide_cd_probe+0x810/0xf40
> >> >> [    5.258086]                                 sp=e00000130307fd50 bsp=e000001303078e90
> >> >> [    5.273709]  [<a00000010050a510>] generic_ide_probe+0x70/0xa0
> >> >> [    5.273711]                                 sp=e00000130307fdc0 bsp=e000001303078e70
> >> >> [    5.287774]  [<a0000001004bdaf0>] driver_probe_device+0x190/0x3a0
> >> >> [    5.287775]                                 sp=e00000130307fdc0 bsp=e000001303078e28
> >> >> [    5.302163]  [<a0000001004bdd80>] __driver_attach+0x80/0xe0
> >> >> [    5.302164]                                 sp=e00000130307fdc0 bsp=e000001303078de8
> >> >> [    5.316032]  [<a0000001004bc5a0>] bus_for_each_dev+0xc0/0x140
> >> >> [    5.316034]                                 sp=e00000130307fdc0 bsp=e000001303078db0
> >> >> [    5.330072]  [<a0000001004bd700>] driver_attach+0x40/0x60
> >> >> [    5.330074]                                 sp=e00000130307fde0 bsp=e000001303078d90
> >> >> [    5.343761]  [<a0000001004bd290>] bus_add_driver+0x370/0x4a0
> >> >> [    5.343763]                                 sp=e00000130307fde0 bsp=e000001303078d48
> >> >> [    5.357720]  [<a0000001004be3d0>] driver_register+0xd0/0x340
> >> >> [    5.357721]                                 sp=e00000130307fde0 bsp=e000001303078d00
> >> >> [    5.371693]  [<a0000001009a3ea0>] ide_cdrom_init+0x20/0x40
> >> >> [    5.371695]                                 sp=e00000130307fde0 bsp=e000001303078ce8
> >> >> [    5.385475]  [<a00000010000a5a0>] do_one_initcall+0x60/0x380
> >> >> [    5.385477]                                 sp=e00000130307fde0 bsp=e000001303078ca8
> >> >> [    5.399445]  [<a0000001009645b0>] kernel_init+0x370/0x420
> >> >> [    5.399447]                                 sp=e00000130307fe20 bsp=e000001303078c68
> >> >> [    5.413148]  [<a000000100013590>] kernel_thread_helper+0xd0/0x100
> >> >> [    5.413149]                                 sp=e00000130307fe30 bsp=e000001303078c40
> >> >> [    5.427547]  [<a00000010000a4c0>] start_kernel_thread+0x20/0x40
> >> >> [    5.427548]                                 sp=e00000130307fe30 bsp=e000001303078c40
> >> >>
> >> >> We are trying to send a sense command to the device and the buffer we
> >> >> use (which is rq->cmd) is what is unaligned, I believe. I'm not sure how
> >> >> useful I can be going forward...
> >> >
> >> > Borislav/Fujita, any ideas what is going wrong with ide-cd?
> >> >
> >>
> >> I think its the following:
> >>
> >> ide_cdrom_get_capabilities() allocates a struct packet_command cgc on
> >> the stack in order to do cdrom_mode_sense() later on. Since that cmd
> >> is not 4byte aligned as we've seen above and we don't do the alignment
> >> check in ide_cd_queue_pc() similar to cdrom_do_block_pc() (see
> >> 0b6abc17700a7843b165c677da0ac94522f83083), we bust the transfer later.
> >>
> >> I'll cook up something later when I have the time...
> >
> > I'm happy to test any patches (and it should be relatively quick to
> > test).
> 
> Sure, however, the problem is more hairy than I expected - the rq->cmd buffer
> is received through blk_get_request() and it is sometimes 2 bytes aligned and
> not 4. Bart, is it a sensible approach to do an ia64 ifdef that checks the
> alignment of the buffer and turns off drive->io_32bit which is checked in
> ide_output_data() thus doing only out(w|b) accesses on ia64 with misaligned
> buffers?

Well, it sounds kind of like a hack if I have to answer honestly. ;-)

Why not just make rq->cmd always properly aligned in the block layer
i.e. by allocating BLK_MAX_CDB + 2 instead of BLK_MAX_CDB for rq->__cmd
and then doing ALIGN() when setting rq->cmd in blk_rq_init()?

[ I'm sure there exists even better solution... ]

Thanks,
Bart

  reply	other threads:[~2008-08-22 18:36 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-19 22:56 kernel unaligned accesses on IA64 in IDE Nishanth Aravamudan
2008-08-19 22:56 ` Nishanth Aravamudan
2008-08-20  1:39 ` Peter Chubb
2008-08-20  1:39   ` Peter Chubb
2008-08-21 21:28   ` Nishanth Aravamudan
2008-08-21 21:28     ` Nishanth Aravamudan
2008-08-20 14:35 ` Robin Holt
2008-08-20 14:35   ` Robin Holt
2008-08-21 21:31   ` Nishanth Aravamudan
2008-08-21 21:31     ` Nishanth Aravamudan
2008-08-21 21:54     ` Robin Holt
2008-08-21 21:54       ` Robin Holt
2008-08-22  0:39       ` Nishanth Aravamudan
2008-08-22  0:39         ` Nishanth Aravamudan
2008-08-22  1:11         ` Robin Holt
2008-08-22  1:11           ` Robin Holt
2008-08-22 16:45           ` Nishanth Aravamudan
2008-08-22 16:45             ` Nishanth Aravamudan
2008-08-22 10:15         ` Bartlomiej Zolnierkiewicz
2008-08-22 10:15           ` Bartlomiej Zolnierkiewicz
2008-08-22 10:55           ` Boris Petkov
2008-08-22 10:55             ` Boris Petkov
2008-08-22 16:45             ` Nishanth Aravamudan
2008-08-22 16:45               ` Nishanth Aravamudan
2008-08-22 17:29               ` Boris Petkov
2008-08-22 17:29                 ` Boris Petkov
2008-08-22 18:36                 ` Bartlomiej Zolnierkiewicz [this message]
2008-08-22 18:36                   ` Bartlomiej Zolnierkiewicz
2008-08-22 18:51                   ` Luck, Tony
2008-08-22 18:51                     ` Luck, Tony
2008-08-22 19:39                     ` Robin Holt
2008-08-22 19:39                       ` Robin Holt
2008-08-22 20:36                       ` Luck, Tony
2008-08-22 20:36                         ` Luck, Tony
2008-08-22 20:41                         ` Borislav Petkov
2008-08-22 20:41                           ` Borislav Petkov
2008-08-22 20:54                         ` Borislav Petkov
2008-08-22 20:54                           ` Borislav Petkov
2008-08-22 21:38                           ` Nishanth Aravamudan
2008-08-22 21:38                             ` Nishanth Aravamudan
2008-08-22 21:49                             ` Borislav Petkov
2008-08-22 21:49                               ` Borislav Petkov
2008-08-22 21:14                         ` Borislav Petkov
2008-08-22 21:14                           ` Borislav Petkov
2008-08-22 23:02                           ` Nishanth Aravamudan
2008-08-22 23:02                             ` Nishanth Aravamudan
2008-08-22 23:30                             ` Luck, Tony
2008-08-22 23:30                               ` Luck, Tony
2008-08-22 23:33                               ` James Bottomley
2008-08-22 23:33                                 ` James Bottomley
2008-08-22 21:15                         ` James Bottomley
2008-08-22 21:15                           ` James Bottomley
2008-08-25 16:31                           ` Nishanth Aravamudan
2008-08-25 16:31                             ` Nishanth Aravamudan

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=200808222036.00536.bzolnier@gmail.com \
    --to=bzolnier@gmail.com \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=holt@sgi.com \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=nacc@us.ibm.com \
    --cc=petkovbb@gmail.com \
    --cc=tony.luck@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.