From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerome Marchand Subject: Re: [PATCH 02/24] io-controller: Core of the elevator fair queuing Date: Thu, 20 Aug 2009 16:51:46 +0200 Message-ID: <4A8D6302.3080301@redhat.com> References: <1250451046-9966-1-git-send-email-vgoyal@redhat.com> <1250451046-9966-3-git-send-email-vgoyal@redhat.com> <4A8C21DE.1080001@redhat.com> <20090819184142.GD4391@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20090819184142.GD4391@redhat.com> Sender: linux-kernel-owner@vger.kernel.org To: Vivek Goyal Cc: linux-kernel@vger.kernel.org, containers@lists.linux-foundation.org, dm-devel@redhat.com, jens.axboe@oracle.com, ryov@valinux.co.jp, balbir@linux.vnet.ibm.com, righi.andrea@gmail.com, nauman@google.com, dpshah@google.com, lizf@cn.fujitsu.com, mikew@google.com, fchecconi@gmail.com, paolo.valente@unimore.it, fernando@oss.ntt.co.jp, s-uchida@ap.jp.nec.com, taka@valinux.co.jp, guijianfeng@cn.fujitsu.com, jmoyer@redhat.com, dhaval@linux.vnet.ibm.com, m-ikeda@ds.jp.nec.com, agk@redhat.com, akpm@linux-foundation.org, peterz@infradead.org List-Id: dm-devel.ids Vivek Goyal wrote: > On Wed, Aug 19, 2009 at 06:01:34PM +0200, Jerome Marchand wrote: >> Hi Vivek, >> >> Vivek Goyal wrote: >>> o This is core of the io scheduler implemented at elevator layer. This is a mix >>> of cpu CFS scheduler and CFQ IO scheduler. Some of the bits from CFS have >>> to be derived so that we can support hierarchical scheduling. Without >>> cgroups or with-in group, we should essentially get same behavior as CFQ. >>> >>> o This patch only shows non-hierarchical bits. Hierarhical code comes in later >>> patches. >>> >>> o This code is the building base of introducing fair queuing logic in common >>> elevator layer so that it can be used by all the four IO schedulers. >>> +static void enqueue_io_entity(struct io_entity *entity) >>> +{ >>> + struct io_service_tree *st = entity->st; >>> + struct io_sched_data *sd = io_entity_sched_data(entity); >>> + >>> + /* In case task ioprio class changed while entity was off tree */ >>> + io_entity_update_prio(entity); >>> + st->nr_active++; >>> + sd->nr_active++; >>> + entity->on_st = 1; >>> + place_entity(st, entity, 0); >>> + __enqueue_io_entity(st, entity); >>> +} One more thing. io_entity_update_prio(entity) can change entity->st, so we'd better set st after io_entity_update_prio() call. I think it fixes a bug I've seen when changing elevator and ioprio_class at the same time: ------------[ cut here ]------------ kernel BUG at block/elevator-fq.c:1921! invalid opcode: 0000 [#1] SMP last sysfs file: /sys/block/sda/queue/scheduler Modules linked in: ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp ipv6 autofs4 hidp rfcomm l2cap bluetooth rfkill sunrpc dm_mirror dm_region_hash dm_log dm_multipath scsi_dh dm_mod video output sbs sbshc battery ac netconsole lp snd_hda_codec_analog snd_hda_intel snd_hda_codec snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device sg snd_pcm_oss snd_mixer_oss rtc_cmos sr_mod snd_pcm cdrom rtc_core tg3 snd_timer libphy snd serio_raw dcdbas i2c_i801 soundcore button pcspkr rtc_lib snd_page_alloc i2c_core parport_pc parport ata_piix libatasd_mod scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd [last unloaded: microcode] Pid: 9127, comm: change_elevator Tainted: G M (2.6.31-rc6-io-controller-v8 #69) OptiPlex 745 EIP: 0060:[] EFLAGS: 00010286 CPU: 1 EIP is at elv_put_iog+0x61/0x83 EAX: 00000000 EBX: 00000000 ECX: f607c000 EDX: f607c001 ESI: f725d300 EDI: f3b59ee0 EBP: f60e5ec0 ESP: f60e5ebc DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 Process change_elevator (pid: 9127, ti=f60e5000 task=f38083f0 task.ti=f60e5000) Stack: f607c000 f60e5ee8 c05382f5 f70ab480 f6abb368 00000086 f70ab480 00000000 <0> f725d300 f725d334 f38d3cc0 f60e5ef8 c05248f7 f725d30c f6abb368 f60e5f28 <0> c0524a67 00000005 00000000 f725d300 706f6f6e c0820000 f60e5f28 c069e51b Call Trace: [] ? elv_exit_fq_data+0x18d/0x195 [] ? elevator_exit+0x28/0x4e [] ? elv_iosched_store+0x14a/0x1db [] ? mutex_lock_nested+0x2e/0x36 [] ? queue_attr_store+0x50/0x61 [] ? sysfs_write_file+0xb9/0xe4 [] ? sysfs_write_file+0x0/0xe4 [] ? vfs_write+0x84/0xdf [] ? sys_write+0x3b/0x60 [] ? sysenter_do_call+0x12/0x32 Code: 0b eb fe 83 79 48 00 75 18 83 79 4c 00 75 1c 83 79 60 00 75 0c 83 79 64 00 75 10 83 79 78 00 74 04 0f 0b eb fe 83 79 7c 00 74 04 <0f> 0b eb fe 8d 81 ec 00 00 00 ba f6 65 53 c0 e8 e9 41 f3 ff 85 EIP: [] elv_put_iog+0x61/0x83 SS:ESP 0068:f60e5ebc ---[ end trace f56eaa89c47eb665 ]--- With the following patch, I can't reproduce it anymore. Regards, Jerome Signed-off-by: Jerome Marchand --- elevator-fq.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) --- block/elevator-fq.c.orig 2009-08-20 16:39:13.000000000 +0200 +++ block/elevator-fq.c 2009-08-20 14:29:04.000000000 +0200 @@ -519,11 +519,12 @@ __enqueue_io_entity(struct io_service_tr static void enqueue_io_entity(struct io_entity *entity) { - struct io_service_tree *st = entity->st; + struct io_service_tree *st; struct io_sched_data *sd = io_entity_sched_data(entity); /* In case task ioprio class changed while entity was off tree */ io_entity_update_prio(entity); + st = entity->st; st->nr_active++; sd->nr_active++; entity->on_st = 1;