Re: Re : Re: Re: Re : Re: [PATCH] Squashfs: add asynchronous read support
On Mon, Dec 23, 2013 at 12:03:39PM +0900, Chanho Min wrote: > > > > read_pages > > for(page_idx ...) { > > if (!add_to_page_cache_lru)) { <-- 1) > > mapping->a_ops->readpage(filp, page) > > squashfs_readpage > > for (i ...) { 2) Here, 31 pages are inserted into page cache > > grab_cahe_page_nowait <--/ > > add_to_page_cache_lru > > } > > } > > /* > > * 1) will be failed with EEXIST by 2) so every pages other than first > > page > > * in list would be freed > > */ > > page_cache_release(page) > > } > > > > If you see ReadAhead works, it is just by luck as I told you. > > Please simulate it with 64K dd. > You right, This luck happened frequently with 128k dd or my test. Yeah, it was not intented by MM's readahead. If you test it with squashfs 256K compression, you couldn't get a benefit. If you test it with small block size dd like 32K, you couldn't, either. It means it's very fragile. One more thing. Your approach doesn't work page cache has already some sparse page because you are solving only direct page copy part, which couldn't work if we read some sparse page in a file and reclaimed many pages. Please rethink. I already explained what's the problem in your patch. You are ignoring VM's logic. (ex, PageReadahead mark) The squashfs is rather special due to compression FS so if we have no other way, I'd like to support your approach but I pointed out problem in your patch and suggest my solution to overcome the problem. It culd be silly but at least, it's time that you should prove why it's brain-damaged so maintainer will review this thread and decide or suggest easily. :) Here goes again. I suggest it would be better to implment squashfs_readpages and it should work with cache buffer instead of direct page cache so that it could copy from cache buffers to pages passed by MM without freeing them so that it preserves readhead hinted page and would work with VM's readahead well 1. although algorithm in readahead were changed, 2. although you use small block size dd, 3. although you use other compression size of squashfs. Thanks. > > > I understand it but your patch doesn't make it. > > > I think my patch can make it if readahead works normally or luckily. > > Thanks a lot! > Chanho, > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re : Re: Re: Re : Re: [PATCH] Squashfs: add asynchronous read support
> read_pages > for(page_idx ...) { > if (!add_to_page_cache_lru)) { <-- 1) > mapping->a_ops->readpage(filp, page) > squashfs_readpage > for (i ...) { 2) Here, 31 pages are inserted into page cache > grab_cahe_page_nowait <--/ > add_to_page_cache_lru > } > } > /* > * 1) will be failed with EEXIST by 2) so every pages other than first > page > * in list would be freed > */ > page_cache_release(page) > } > > If you see ReadAhead works, it is just by luck as I told you. > Please simulate it with 64K dd. You right, This luck happened frequently with 128k dd or my test. > I understand it but your patch doesn't make it. > I think my patch can make it if readahead works normally or luckily. Thanks a lot! Chanho, -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: Re : Re: [PATCH] Squashfs: add asynchronous read support
On Sat, Dec 21, 2013 at 11:05:51AM +0900, Chanho Min wrote: > > > Please don't break thread. > > You should reply to my mail instead of your original post. > Sorry, It seems to be my mailer issue. I'm trying to fix it. > > > It's a result which isn't what I want to know. > > What I wnat to know is why upper layer issues more I/O per second. > > For example, you read 32K so MM layer will prepare 8 pages to read in but > > at issuing at a first page, squashfs make 32 pages and fill the page cache > > if we assume you use 128K compression so MM layer's already prepared 7 > > page > > would be freed without further I/O and do_generic_file_read will wait for > > completion by lock_page without further I/O queueing. It's not suprising. > > One of page freed is a READA marked page so readahead couldn't work. > > If readahead works, it would be just by luck. Actually, by simulation > > 64K dd, I found readahead logic would be triggered but it's just by luck > > and it's not intended, I think. > MM layer's readahead pages would not be freed immediately. > Squashfs can use them by grab_cache_page_nowait and READA marked page is > available. > Intentional or not, readahead works pretty well. I checked in experiment. read_pages for(page_idx ...) { if (!add_to_page_cache_lru)) { <-- 1) mapping->a_ops->readpage(filp, page) squashfs_readpage for (i ...) { 2) Here, 31 pages are inserted into page cache grab_cahe_page_nowait <--/ add_to_page_cache_lru } } /* * 1) will be failed with EEXIST by 2) so every pages other than first page * in list would be freed */ page_cache_release(page) } If you see ReadAhead works, it is just by luck as I told you. Please simulate it with 64K dd. > > > If first issued I/O complete, squashfs decompress the I/O with 128K pages > > so all 4 iteration(128K/32K) would be hit in page cache. > > If all 128K hit in page cache, mm layer start to issue next I/O and > > repeat above logic until you ends up reading all file size. > > So my opition is that upper layer wouldn't issue more I/O logically. > > If it worked, it's not what we expect but side-effect. > > > > That's why I'd like to know what's your thought for increasing IOPS. > > Please, could you say your thought why IOPS increased, not a result > > on low level driver? > It is because readahead can works asynchronously in background. > Suppose that you read a large file by 128k partially and contiguously > like "dd bs=128k". Two IOs can be issued per 128k reading, > First IO is for intended pages, second IO is for readahead. > If first IO hit in cache thank to previous readahead, no wait for IO > completion > is needed, because intended page is up-to-date already. > But, current squashfs waits for second IO's completion unnecessarily. > That is one of reason that we should move page's up-to-date > to the asynchronous area like my patch. I understand it but your patch doesn't make it. > > > Anyway, in my opinion, we should take care of MM layer's readahead for > > enhance sequential I/O. For it, we should use buffer pages passed by MM > > instead of freeing them and allocating new pages in squashfs. > > IMHO, it would be better to implement squashfs_readpages but my insight > > is very weak so I guess Phillip will give more good idea/insight about > > the issue. > That's a good point. Also, I think my patch is another way which can be > implemented > without significant impact on current implementation and I wait for Phillip's > comment. > > Thanks > Chanho > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: Re : Re: [PATCH] Squashfs: add asynchronous read support
On Sat, Dec 21, 2013 at 11:05:51AM +0900, Chanho Min wrote: Please don't break thread. You should reply to my mail instead of your original post. Sorry, It seems to be my mailer issue. I'm trying to fix it. It's a result which isn't what I want to know. What I wnat to know is why upper layer issues more I/O per second. For example, you read 32K so MM layer will prepare 8 pages to read in but at issuing at a first page, squashfs make 32 pages and fill the page cache if we assume you use 128K compression so MM layer's already prepared 7 page would be freed without further I/O and do_generic_file_read will wait for completion by lock_page without further I/O queueing. It's not suprising. One of page freed is a READA marked page so readahead couldn't work. If readahead works, it would be just by luck. Actually, by simulation 64K dd, I found readahead logic would be triggered but it's just by luck and it's not intended, I think. MM layer's readahead pages would not be freed immediately. Squashfs can use them by grab_cache_page_nowait and READA marked page is available. Intentional or not, readahead works pretty well. I checked in experiment. read_pages for(page_idx ...) { if (!add_to_page_cache_lru)) { -- 1) mapping-a_ops-readpage(filp, page) squashfs_readpage for (i ...) { 2) Here, 31 pages are inserted into page cache grab_cahe_page_nowait --/ add_to_page_cache_lru } } /* * 1) will be failed with EEXIST by 2) so every pages other than first page * in list would be freed */ page_cache_release(page) } If you see ReadAhead works, it is just by luck as I told you. Please simulate it with 64K dd. If first issued I/O complete, squashfs decompress the I/O with 128K pages so all 4 iteration(128K/32K) would be hit in page cache. If all 128K hit in page cache, mm layer start to issue next I/O and repeat above logic until you ends up reading all file size. So my opition is that upper layer wouldn't issue more I/O logically. If it worked, it's not what we expect but side-effect. That's why I'd like to know what's your thought for increasing IOPS. Please, could you say your thought why IOPS increased, not a result on low level driver? It is because readahead can works asynchronously in background. Suppose that you read a large file by 128k partially and contiguously like dd bs=128k. Two IOs can be issued per 128k reading, First IO is for intended pages, second IO is for readahead. If first IO hit in cache thank to previous readahead, no wait for IO completion is needed, because intended page is up-to-date already. But, current squashfs waits for second IO's completion unnecessarily. That is one of reason that we should move page's up-to-date to the asynchronous area like my patch. I understand it but your patch doesn't make it. Anyway, in my opinion, we should take care of MM layer's readahead for enhance sequential I/O. For it, we should use buffer pages passed by MM instead of freeing them and allocating new pages in squashfs. IMHO, it would be better to implement squashfs_readpages but my insight is very weak so I guess Phillip will give more good idea/insight about the issue. That's a good point. Also, I think my patch is another way which can be implemented without significant impact on current implementation and I wait for Phillip's comment. Thanks Chanho -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re : Re: Re: Re : Re: [PATCH] Squashfs: add asynchronous read support
read_pages for(page_idx ...) { if (!add_to_page_cache_lru)) { -- 1) mapping-a_ops-readpage(filp, page) squashfs_readpage for (i ...) { 2) Here, 31 pages are inserted into page cache grab_cahe_page_nowait --/ add_to_page_cache_lru } } /* * 1) will be failed with EEXIST by 2) so every pages other than first page * in list would be freed */ page_cache_release(page) } If you see ReadAhead works, it is just by luck as I told you. Please simulate it with 64K dd. You right, This luck happened frequently with 128k dd or my test. I understand it but your patch doesn't make it. I think my patch can make it if readahead works normally or luckily. Thanks a lot! Chanho, -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re : Re: Re: Re : Re: [PATCH] Squashfs: add asynchronous read support
On Mon, Dec 23, 2013 at 12:03:39PM +0900, Chanho Min wrote: read_pages for(page_idx ...) { if (!add_to_page_cache_lru)) { -- 1) mapping-a_ops-readpage(filp, page) squashfs_readpage for (i ...) { 2) Here, 31 pages are inserted into page cache grab_cahe_page_nowait --/ add_to_page_cache_lru } } /* * 1) will be failed with EEXIST by 2) so every pages other than first page * in list would be freed */ page_cache_release(page) } If you see ReadAhead works, it is just by luck as I told you. Please simulate it with 64K dd. You right, This luck happened frequently with 128k dd or my test. Yeah, it was not intented by MM's readahead. If you test it with squashfs 256K compression, you couldn't get a benefit. If you test it with small block size dd like 32K, you couldn't, either. It means it's very fragile. One more thing. Your approach doesn't work page cache has already some sparse page because you are solving only direct page copy part, which couldn't work if we read some sparse page in a file and reclaimed many pages. Please rethink. I already explained what's the problem in your patch. You are ignoring VM's logic. (ex, PageReadahead mark) The squashfs is rather special due to compression FS so if we have no other way, I'd like to support your approach but I pointed out problem in your patch and suggest my solution to overcome the problem. It culd be silly but at least, it's time that you should prove why it's brain-damaged so maintainer will review this thread and decide or suggest easily. :) Here goes again. I suggest it would be better to implment squashfs_readpages and it should work with cache buffer instead of direct page cache so that it could copy from cache buffers to pages passed by MM without freeing them so that it preserves readhead hinted page and would work with VM's readahead well 1. although algorithm in readahead were changed, 2. although you use small block size dd, 3. although you use other compression size of squashfs. Thanks. I understand it but your patch doesn't make it. I think my patch can make it if readahead works normally or luckily. Thanks a lot! Chanho, -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: Re : Re: [PATCH] Squashfs: add asynchronous read support
> Please don't break thread. > You should reply to my mail instead of your original post. Sorry, It seems to be my mailer issue. I'm trying to fix it. > It's a result which isn't what I want to know. > What I wnat to know is why upper layer issues more I/O per second. > For example, you read 32K so MM layer will prepare 8 pages to read in but > at issuing at a first page, squashfs make 32 pages and fill the page cache > if we assume you use 128K compression so MM layer's already prepared 7 > page > would be freed without further I/O and do_generic_file_read will wait for > completion by lock_page without further I/O queueing. It's not suprising. > One of page freed is a READA marked page so readahead couldn't work. > If readahead works, it would be just by luck. Actually, by simulation > 64K dd, I found readahead logic would be triggered but it's just by luck > and it's not intended, I think. MM layer's readahead pages would not be freed immediately. Squashfs can use them by grab_cache_page_nowait and READA marked page is available. Intentional or not, readahead works pretty well. I checked in experiment. > If first issued I/O complete, squashfs decompress the I/O with 128K pages > so all 4 iteration(128K/32K) would be hit in page cache. > If all 128K hit in page cache, mm layer start to issue next I/O and > repeat above logic until you ends up reading all file size. > So my opition is that upper layer wouldn't issue more I/O logically. > If it worked, it's not what we expect but side-effect. > > That's why I'd like to know what's your thought for increasing IOPS. > Please, could you say your thought why IOPS increased, not a result > on low level driver? It is because readahead can works asynchronously in background. Suppose that you read a large file by 128k partially and contiguously like "dd bs=128k". Two IOs can be issued per 128k reading, First IO is for intended pages, second IO is for readahead. If first IO hit in cache thank to previous readahead, no wait for IO completion is needed, because intended page is up-to-date already. But, current squashfs waits for second IO's completion unnecessarily. That is one of reason that we should move page's up-to-date to the asynchronous area like my patch. > Anyway, in my opinion, we should take care of MM layer's readahead for > enhance sequential I/O. For it, we should use buffer pages passed by MM > instead of freeing them and allocating new pages in squashfs. > IMHO, it would be better to implement squashfs_readpages but my insight > is very weak so I guess Phillip will give more good idea/insight about > the issue. That's a good point. Also, I think my patch is another way which can be implemented without significant impact on current implementation and I wait for Phillip's comment. Thanks Chanho -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: Re : Re: [PATCH] Squashfs: add asynchronous read support
Please don't break thread. You should reply to my mail instead of your original post. Sorry, It seems to be my mailer issue. I'm trying to fix it. It's a result which isn't what I want to know. What I wnat to know is why upper layer issues more I/O per second. For example, you read 32K so MM layer will prepare 8 pages to read in but at issuing at a first page, squashfs make 32 pages and fill the page cache if we assume you use 128K compression so MM layer's already prepared 7 page would be freed without further I/O and do_generic_file_read will wait for completion by lock_page without further I/O queueing. It's not suprising. One of page freed is a READA marked page so readahead couldn't work. If readahead works, it would be just by luck. Actually, by simulation 64K dd, I found readahead logic would be triggered but it's just by luck and it's not intended, I think. MM layer's readahead pages would not be freed immediately. Squashfs can use them by grab_cache_page_nowait and READA marked page is available. Intentional or not, readahead works pretty well. I checked in experiment. If first issued I/O complete, squashfs decompress the I/O with 128K pages so all 4 iteration(128K/32K) would be hit in page cache. If all 128K hit in page cache, mm layer start to issue next I/O and repeat above logic until you ends up reading all file size. So my opition is that upper layer wouldn't issue more I/O logically. If it worked, it's not what we expect but side-effect. That's why I'd like to know what's your thought for increasing IOPS. Please, could you say your thought why IOPS increased, not a result on low level driver? It is because readahead can works asynchronously in background. Suppose that you read a large file by 128k partially and contiguously like dd bs=128k. Two IOs can be issued per 128k reading, First IO is for intended pages, second IO is for readahead. If first IO hit in cache thank to previous readahead, no wait for IO completion is needed, because intended page is up-to-date already. But, current squashfs waits for second IO's completion unnecessarily. That is one of reason that we should move page's up-to-date to the asynchronous area like my patch. Anyway, in my opinion, we should take care of MM layer's readahead for enhance sequential I/O. For it, we should use buffer pages passed by MM instead of freeing them and allocating new pages in squashfs. IMHO, it would be better to implement squashfs_readpages but my insight is very weak so I guess Phillip will give more good idea/insight about the issue. That's a good point. Also, I think my patch is another way which can be implemented without significant impact on current implementation and I wait for Phillip's comment. Thanks Chanho -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re : Re: [PATCH] Squashfs: add asynchronous read support
Hello, Please don't break thread. You should reply to my mail instead of your original post. On Wed, Dec 18, 2013 at 01:29:37PM +0900, Chanho Min wrote: > > > I did test it on x86 with USB stick and ARM with eMMC on my Nexus 4. > > In experiment, I couldn't see much gain like you both system and even it > > was regressed at bs=32k test, maybe workqueue allocation/schedule of work > > per I/O. > > Your test is rather special or what I am missing? > Can you specify your test result on ARM with eMMC. Sure. before after 32K 3.6M 3.4M 64K 6.3M 8.2M 128K11.4M11.7M 160K13.6M13.8M 256K19.8M19M 288K21.3M20.8M > > > Before that, I'd like to know fundamental reason why your implementation > > for asynchronous read enhance. At a first glance, I thought it's caused by > > readahead from MM layer but when I read code, I found I was wrong. > > MM's readahead logic works based on PageReadahead marker but squashfs > > invalidates by grab_cache_page_nowait so it wouldn't work as we expected. > > > > Another possibility is block I/O merging in block layder by plugging logic, > > which was what I tried a few month ago although implementation was really > > bad. But it wouldn't work with your patch because do_generic_file_read > > will unplug block layer by lock_page without merging enough I/O. > > > > So, what do you think real actuator for enhance your experiment? > > Then, I could investigate why I can't get a benefit. > Currently, squashfs adds request to the block device queue synchronously with > wait for competion. mmc takes this request one by one and push them to host > driver, > But it allows mmc to be idle frequently. This patch allows to add block > requset > asynchrously without wait for competion, mmcqd can fetch a lot of request > from block > at a time. As a result, mmcqd get busy and use a more bandwidth of mmc. > For test, I added two count variables in mmc_queue_thread as bellows > and tested same dd transfer. > > static int mmc_queue_thread(void *d) > { > .. > do { > if (req || mq->mqrq_prev->req) { > fetch++; > } else { > idle++; > } > } while (1); > .. > } > > without patch: > fetch: 920, idle: 460 > > with patch > fetch: 918, idle: 40 It's a result which isn't what I want to know. What I wnat to know is why upper layer issues more I/O per second. For example, you read 32K so MM layer will prepare 8 pages to read in but at issuing at a first page, squashfs make 32 pages and fill the page cache if we assume you use 128K compression so MM layer's already prepared 7 page would be freed without further I/O and do_generic_file_read will wait for completion by lock_page without further I/O queueing. It's not suprising. One of page freed is a READA marked page so readahead couldn't work. If readahead works, it would be just by luck. Actually, by simulation 64K dd, I found readahead logic would be triggered but it's just by luck and it's not intended, I think. If first issued I/O complete, squashfs decompress the I/O with 128K pages so all 4 iteration(128K/32K) would be hit in page cache. If all 128K hit in page cache, mm layer start to issue next I/O and repeat above logic until you ends up reading all file size. So my opition is that upper layer wouldn't issue more I/O logically. If it worked, it's not what we expect but side-effect. That's why I'd like to know what's your thought for increasing IOPS. Please, could you say your thought why IOPS increased, not a result on low level driver? Anyway, in my opinion, we should take care of MM layer's readahead for enhance sequential I/O. For it, we should use buffer pages passed by MM instead of freeing them and allocating new pages in squashfs. IMHO, it would be better to implement squashfs_readpages but my insight is very weak so I guess Phillip will give more good idea/insight about the issue. Thanks! > > Thanks > Chanho. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re : Re: [PATCH] Squashfs: add asynchronous read support
> I did test it on x86 with USB stick and ARM with eMMC on my Nexus 4. > In experiment, I couldn't see much gain like you both system and even it > was regressed at bs=32k test, maybe workqueue allocation/schedule of work > per I/O. > Your test is rather special or what I am missing? Can you specify your test result on ARM with eMMC. > Before that, I'd like to know fundamental reason why your implementation > for asynchronous read enhance. At a first glance, I thought it's caused by > readahead from MM layer but when I read code, I found I was wrong. > MM's readahead logic works based on PageReadahead marker but squashfs > invalidates by grab_cache_page_nowait so it wouldn't work as we expected. > > Another possibility is block I/O merging in block layder by plugging logic, > which was what I tried a few month ago although implementation was really > bad. But it wouldn't work with your patch because do_generic_file_read > will unplug block layer by lock_page without merging enough I/O. > > So, what do you think real actuator for enhance your experiment? > Then, I could investigate why I can't get a benefit. Currently, squashfs adds request to the block device queue synchronously with wait for competion. mmc takes this request one by one and push them to host driver, But it allows mmc to be idle frequently. This patch allows to add block requset asynchrously without wait for competion, mmcqd can fetch a lot of request from block at a time. As a result, mmcqd get busy and use a more bandwidth of mmc. For test, I added two count variables in mmc_queue_thread as bellows and tested same dd transfer. static int mmc_queue_thread(void *d) { .. do { if (req || mq->mqrq_prev->req) { fetch++; } else { idle++; } } while (1); .. } without patch: fetch: 920, idle: 460 with patch fetch: 918, idle: 40 Thanks Chanho. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re : Re: [PATCH] Squashfs: add asynchronous read support
I did test it on x86 with USB stick and ARM with eMMC on my Nexus 4. In experiment, I couldn't see much gain like you both system and even it was regressed at bs=32k test, maybe workqueue allocation/schedule of work per I/O. Your test is rather special or what I am missing? Can you specify your test result on ARM with eMMC. Before that, I'd like to know fundamental reason why your implementation for asynchronous read enhance. At a first glance, I thought it's caused by readahead from MM layer but when I read code, I found I was wrong. MM's readahead logic works based on PageReadahead marker but squashfs invalidates by grab_cache_page_nowait so it wouldn't work as we expected. Another possibility is block I/O merging in block layder by plugging logic, which was what I tried a few month ago although implementation was really bad. But it wouldn't work with your patch because do_generic_file_read will unplug block layer by lock_page without merging enough I/O. So, what do you think real actuator for enhance your experiment? Then, I could investigate why I can't get a benefit. Currently, squashfs adds request to the block device queue synchronously with wait for competion. mmc takes this request one by one and push them to host driver, But it allows mmc to be idle frequently. This patch allows to add block requset asynchrously without wait for competion, mmcqd can fetch a lot of request from block at a time. As a result, mmcqd get busy and use a more bandwidth of mmc. For test, I added two count variables in mmc_queue_thread as bellows and tested same dd transfer. static int mmc_queue_thread(void *d) { .. do { if (req || mq-mqrq_prev-req) { fetch++; } else { idle++; } } while (1); .. } without patch: fetch: 920, idle: 460 with patch fetch: 918, idle: 40 Thanks Chanho. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re : Re: [PATCH] Squashfs: add asynchronous read support
Hello, Please don't break thread. You should reply to my mail instead of your original post. On Wed, Dec 18, 2013 at 01:29:37PM +0900, Chanho Min wrote: I did test it on x86 with USB stick and ARM with eMMC on my Nexus 4. In experiment, I couldn't see much gain like you both system and even it was regressed at bs=32k test, maybe workqueue allocation/schedule of work per I/O. Your test is rather special or what I am missing? Can you specify your test result on ARM with eMMC. Sure. before after 32K 3.6M 3.4M 64K 6.3M 8.2M 128K11.4M11.7M 160K13.6M13.8M 256K19.8M19M 288K21.3M20.8M Before that, I'd like to know fundamental reason why your implementation for asynchronous read enhance. At a first glance, I thought it's caused by readahead from MM layer but when I read code, I found I was wrong. MM's readahead logic works based on PageReadahead marker but squashfs invalidates by grab_cache_page_nowait so it wouldn't work as we expected. Another possibility is block I/O merging in block layder by plugging logic, which was what I tried a few month ago although implementation was really bad. But it wouldn't work with your patch because do_generic_file_read will unplug block layer by lock_page without merging enough I/O. So, what do you think real actuator for enhance your experiment? Then, I could investigate why I can't get a benefit. Currently, squashfs adds request to the block device queue synchronously with wait for competion. mmc takes this request one by one and push them to host driver, But it allows mmc to be idle frequently. This patch allows to add block requset asynchrously without wait for competion, mmcqd can fetch a lot of request from block at a time. As a result, mmcqd get busy and use a more bandwidth of mmc. For test, I added two count variables in mmc_queue_thread as bellows and tested same dd transfer. static int mmc_queue_thread(void *d) { .. do { if (req || mq-mqrq_prev-req) { fetch++; } else { idle++; } } while (1); .. } without patch: fetch: 920, idle: 460 with patch fetch: 918, idle: 40 It's a result which isn't what I want to know. What I wnat to know is why upper layer issues more I/O per second. For example, you read 32K so MM layer will prepare 8 pages to read in but at issuing at a first page, squashfs make 32 pages and fill the page cache if we assume you use 128K compression so MM layer's already prepared 7 page would be freed without further I/O and do_generic_file_read will wait for completion by lock_page without further I/O queueing. It's not suprising. One of page freed is a READA marked page so readahead couldn't work. If readahead works, it would be just by luck. Actually, by simulation 64K dd, I found readahead logic would be triggered but it's just by luck and it's not intended, I think. If first issued I/O complete, squashfs decompress the I/O with 128K pages so all 4 iteration(128K/32K) would be hit in page cache. If all 128K hit in page cache, mm layer start to issue next I/O and repeat above logic until you ends up reading all file size. So my opition is that upper layer wouldn't issue more I/O logically. If it worked, it's not what we expect but side-effect. That's why I'd like to know what's your thought for increasing IOPS. Please, could you say your thought why IOPS increased, not a result on low level driver? Anyway, in my opinion, we should take care of MM layer's readahead for enhance sequential I/O. For it, we should use buffer pages passed by MM instead of freeing them and allocating new pages in squashfs. IMHO, it would be better to implement squashfs_readpages but my insight is very weak so I guess Phillip will give more good idea/insight about the issue. Thanks! Thanks Chanho. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/