[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[lvm-devel] [PATCH LVM2] Prefer small areas if possible



Hi,

This patch fixes the mismatch between the code and the comment
regarding the preference of the smaller area possible.


Comment in the code suggests choosing the smallest possible PE
to satisfy the request, which sounds reasonable.
However, the "goto next_pv" at the end prevents it to happen:
if PV with the sufficient area is found, don't search the PV further.
(Note that PV area in the PV is sorted by size.)

The comment is this (In lib/metadata/lv_manip.c:_find_parallel_space()):
   /*
    * Put the smallest area of each PV that is at least the
    * size we need into areas array.  If there isn't one
    * that fits completely and we're allowed more than one
    * LV segment, then take the largest remaining instead.
    */

The logic was:
  foreach pv {
     foreach pv_area { # pv_area is sorted by size (large to small)
        if the pv_area is smaller than requested {
           if there is already one candidate found in this pv {
              use the candidate and goto next pv
              <i.e. the smallest possible area is chosen>
           } else if split is allowed {
              # at this point, there was no area fits completely
              select the pv_area as a candidate
              goto next pv_area
              <i.e. the largest area is chosen>
           } else
              no candidate from this pv, goto next pv
        } else {
           select the pv_area as a candidate
           goto next pv
        }
     }
  }
and the patch removes the goto at the end.


Patch applicable to LVM2 2.02.16.

Shell script to reproduce the problem and the sample metadata
is attached for reference.
You can reproduce the problem with just executing the script
without additional parameters.
Also, additional test scripts are included.

-- 
Jun'ichi Nomura, NEC Corporation of America
Comment in the code suggests choosing the smallest possible PE
to satisfy the request, which sounds reasonable.
However, the "goto next_pv" at the end prevents it to happen:
if PV with the sufficient area is found, don't search the PV further.
(Note that PV area in the PV is sorted by size.)

The comment is this (In lib/metadata/lv_manip.c:_find_parallel_space()):
   /*
    * Put the smallest area of each PV that is at least the
    * size we need into areas array.  If there isn't one
    * that fits completely and we're allowed more than one
    * LV segment, then take the largest remaining instead.
    */

Patch applicable to LVM2 2.02.16.

---
 lib/metadata/lv_manip.c |    2 --
 1 file changed, 2 deletions(-)

Index: LVM2.cvs/lib/metadata/lv_manip.c
===================================================================
--- LVM2.cvs.orig/lib/metadata/lv_manip.c	2006-12-13 00:08:42.000000000 -0500
+++ LVM2.cvs/lib/metadata/lv_manip.c	2006-12-13 00:08:44.000000000 -0500
@@ -1007,8 +1007,6 @@ static int _find_parallel_space(struct a
 				}
 
 				areas[ix + ix_offset - 1] = pva;
-
-				goto next_pv;
 			}
 		next_pv:
 			if (ix >= areas_size)

Attachment: lvm2-prefer-smaller-area.sh
Description: Bourne shell script

# Generated by LVM2: Tue Dec 12 20:47:36 2006

contents = "Text Format Volume Group"
version = 1

description = "Created *before* executing 'lvcreate -l1 -n lv0 vg0'"

creation_host = "tetsuo.lab"	# Linux tetsuo.lab 2.6.18-1.2839.el5 #1 SMP Wed Dec 6 01:01:57 EST 2006 i686
creation_time = 1165974456	# Tue Dec 12 20:47:36 2006

vg0 {
	id = "BDsm59-jFZk-3sdc-sygD-M2KR-rlWF-Aa1gx6"
	seqno = 1
	status = ["RESIZEABLE", "READ", "WRITE"]
	extent_size = 8			# 4 Kilobytes
	max_lv = 0
	max_pv = 0

	physical_volumes {

		pv0 {
			id = "v3J09Q-QQUj-9edC-qVu3-g0Ur-8npX-OYYYQ7"
			device = "/dev/mapper/pv0"	# Hint only

			status = ["ALLOCATABLE"]
			dev_size = 40960	# 20 Megabytes
			pe_start = 384
			pe_count = 5072	# 19.8125 Megabytes
		}
	}

}

Attachment: lvm2-prefer-smaller-area-appendix1.sh
Description: Bourne shell script

Attachment: lvm2-prefer-smaller-area-appendix2.sh
Description: Bourne shell script


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]